Объявляю ошибку вида if (x = 42) вымирающей и заношу её в Красную книгу C и C++ багов

30 Май 2024

Автор: Андрей Карпов

Если спросить программиста, какие баги чаще всего можно встретить в C и C++ коде, он назовёт разыменование нулевого указателя, неопределённое поведение, выход за границу массива и другие, на его взгляд, типовые паттерны ошибок. Скорее всего, он назовёт и случайное присваивание в условии. Но действительно ли эта ошибка распространена в наше время?

Так уж сложилось, что в языке C и C++ для оператора присваивания используется символ =, а для сравнения ==. Как следствие, возможны опечатки, когда вместо сравнения пишут = и получают компилируемый, но неправильно работающий код.

Выглядят эти ошибки так:

int abcd = foo();
if (abcd = -1)

Происходят сразу две неприятности:

Условие всегда истинно или ложно в зависимости от того, что присваивается переменной.
Потеряно правильное значение. Если переменная abcd далее используется ещё где-то, то её значение окажется неверным (испорченным).

Это настолько простые, но при этом реальные ошибки, что они получили популярность среди программистов. Фильмы, конечно, про такие баги не снимают, но мемы вполне себе присутствуют.

Причём эти ошибки именно так и выглядят. Например, следующий код в своё время я нашёл в проекте AMT SDK:

if (status = true)
{
  PrintSuccess();
}

Ради этого бага даже была придумана нотация Йоды, когда константа пишется слева от оператора сравнения:

if (-1 == abcd)

Такой стиль призван предотвратить опечатку. Если программист вместо сравнения напишет =, то код не скомпилируется.

Впрочем, эта нотация не прижилась на практике. По крайней мере в открытых проектах я встречал её очень редко.

Мне, кстати, она тоже не нравится. Во-первых, она немного усложняет чтение кода, так как вначале ты смотришь на константу и только потом понимаешь, о чём вообще идёт речь, и какая сущность проверяется.

Во-вторых, такой стиль написания кода не поможет, если между собой сравниваются две переменные:

int x = getx();
int y = gety();
if (x = y)

На мой взгляд, лучше объявлять все переменные, которые дальше используются только для чтения, как const. Это уменьшает вероятность их случайной модификации из-за опечаток или неудачного рефакторинга.

const int abcd = foo();
if (abcd = -1)            // Ошибка компиляции
{
  const int x = getx();
  const int y = gety();
  if (x = y)              // Ошибка компиляции

Я немного отвлёкся. В начале я говорил что-то про редкость этих багов и Красную книгу. Вернёмся к самому важному.

Я готовил доклад для одной конференции, где затронул тему того, насколько ожидания встретить какую-то ошибку в коде совпадают с реальностью. Чтобы не быть голословным, я провёл небольшое исследование нашей коллекции багов, найденных за несколько лет написания статей про проверку открытых проектов.

Где-то ожидания совпадают с реальностью. Например, программисты предполагают, что в C и C++ коде много багов, связанных с разыменованием нулевого указателя и выходом за границу массива. И действительно, таких багов много.

Где-то ожидания не совпадают с реальностью. Программист с большой вероятностью скажет про деление на ноль, но на самом деле таких ошибок мало. По крайней мере, в сравнении со многими другими ошибками.

Естественно, я не мог не посмотреть, как обстоит дело с ошибками присваивания в условиях. По ожиданиям, их должна быть цела гора.

Здесь меня ждало открытие. Такие ошибки действительно присутствуют в открытых проектах, хотя их и немного. В нашей коллекции выписаны только 14 случаев. Самое интересное: последнюю ошибку мы выписали в 2016 году!

Удивительно, насколько завышенные ожидания, а ошибок-то и нет!

Давайте ещё раз чуть подробнее про статистику. Анализатор PVS-Studio выявляет ошибки присваивания в условии с помощью диагностического правила V559. Оно появилось в PVS-Studio в версии 4.12, которая вышла 7 февраля 2011 года.

Какое-то время анализатор находил эти ошибки, но постепенно их становилось всё меньше. Последняя найденная нами подобная ошибка датируется 2016 годом. И вот уже как восемь лет они нам не попадаются, хотя мы по-прежнему активно пишем статьи про проверку открытых проектов.

Мне кажется это удивительным и приятным примером, когда какой-то вид багов исчезает благодаря развитию инструментария программистов. Я не думаю, что такие ошибки исчезли из-за того, что все про них знают и теперь аккуратно пишут код. Как показывают наши наблюдения, сейчас опечатки делают так же часто, как и 10 лет назад.

Так что, скорее всего, дело именно в инструментарии. Вначале такие ошибки обнаруживали статические анализаторы. Затем подтянулись компиляторы. В общем, теперь не осталось компиляторов, которые промолчат, встретив присваивание в условии. Напиши сейчас такой код, и тебе прилетят предупреждения со всех сторон: среда разработки подсветит код, компиляторы и статические анализаторы выдадут предупреждения. Мимо не пройдёшь. Красота!

Конечно, я не говорю, что теперь вообще нет программ с такими багами. Есть, просто сами эти баги стали редкостью. Подтверждением этому как раз и служит моё наблюдение, что мы давно их не встречаем, проверяя различные проекты.

Кстати, а не значит ли это, что скоро статические анализаторы станут не нужны, раз компиляторы выдают всё больше предупреждений? Конечно, нет. Развитие анализаторов тоже не стоит на месте. Они учатся находить всё более сложные и заковыристые ошибки. В их арсенале такие технологии, как межпроцедурный и межмодульный анализы, анализ потока данных, символьные вычисления, taint-анализ, и так далее. Вдобавок они могут позволить себе работать не так быстро, как компиляторы, и потреблять больше памяти. Имея это преимущество, они проводят более глубокие и сложные виды анализа.

Можно сказать, статические анализаторы задают вектор развития компиляторов в плане выявления багов. Мы даже замечали, как диагностические правила из PVS-Studio перекочёвывают в компиляторы. Наша команда гордится, что PVS-Studio является для кого-то примером. Пусть хорошие диагностические правила появляются в других инструментах — ошибок станет меньше. А мы тем временем ещё новых правил изобретём. В конце концов, должен же кто-то находить ошибки в компиляторах :)

auto FirstSemiPos = Line.find(';');
if (FirstSemiPos == std::string::npos)
  continue;

auto SecondSemiPos = Line.find(';', FirstSemiPos + 1);
if (FirstSemiPos == std::string::npos)  // <= Следует проверить SecondSemiPos
  continue;

Это пример опечатки в коде LLVM, найденной анализатором PVS-Studio благодаря предупреждению: V547 Expression 'FirstSemiPos == std::string::npos' is always false. UnicodeNameMappingGenerator.cpp 46.

Посмотреть на другие баги в компиляторах.

Напоследок ещё раз вернёмся к присваиванию в условии:

int abcd;
while (abcd = foo())

Что если автор кода действительно хочет одновременно выполнить присваивание и проверку?

Первый вариант. Если вы пишете на C++ и переменная нужна только внутри тела инструкции if/while, то можно объявить её прямо в условии:

while (int abcd = foo())

Сразу очевидно, что здесь объявление переменной с инициализацией и последующей проверкой.

Второй вариант. Есть негласное соглашение между всеми (или почти всеми) разработчиками компиляторов и анализаторов кода, что дополнительные скобочки являются признаком того, что всё правильно.

while ((abcd = foo()))

Если программист так написал, то считается, что он говорит: "Я знаю, что делаю, этот код правильный". В этом случае компиляторы и анализаторы не выдают предупреждение.