Как PVS-Studio оказался внимательнее, чем три с половиной программиста

    Как PVS-Studio оказался внимательнее, чем три с половиной программиста

    PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь оказался внимательнее нескольких человек.

    Нам в поддержку написал пользователь, утверждая, что анализатор выдаёт сразу четыре ложных срабатывания на одну строчку кода. Письмо, написанное в поддержку, изначально попало к Евгению Рыжкову, который, бегло прочитав его и не заметив аномального в фидбеке, сразу переслал его ведущему разработчику Святославу Размыслову. Евгений не всматривался в код, так что будет честно посчитать его только за половину программиста :).


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

    И только когда Святослав начал делать синтетические примеры, чтобы подробно расписать проблему в багтрекере, на него снизошло озарение. Давайте теперь посмотрим, сможете ли вы быстро найти причину, почему анализатор выдаёт 4 сообщения.

    Вот текст письма, публикуемый с разрешения автора. И поясняющая картинка, которая была прикреплена к письму.

    V560 warnings here are all false. Running with most recent version of PVS-Studio for personal use. Basically, «IF» statement is correct. Outer one is done for speed — inner ones are still needed and non are always true or false.

    Письмо


    Теперь, уважаемый читатель, ваше время проверить себя! Видите ошибку?

    Ваше время проявить внимательность. А единорог пока немного подождёт.

    Единорог-ждун


    После вводной части статьи, скорее всего многие нашли ошибку. Когда настроен найти ошибку, она находится. Намного сложнее заметить ляп после прочтения письма, где это называется «ложными срабатываниями» :).

    Теперь пояснение для тех, кто поленился искать баг. Ещё раз рассмотрим условие:

    if (!((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
         ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
         ((ch >= 0x0FF41) && (ch <= 0x0FF5A)))

    Автор кода планировал проверить, что символ не входит ни в один из трёх диапазонов.

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

    Если выполнилось условие:

    !((ch >= 0x0FF10) && (ch <= 0x0FF19))

    то вычисление выражения прерывается согласно short-circuit evaluation. Если условие не выполняется, то значение переменной ch лежит в диапазоне [0xFF10..0xFF19]. Соответственно, четыре дальнейших сравнения не имеют смысла. Все они будут ложными или истинными.

    Ещё раз. Смотрите, если ch лежит в диапазоне [0xFF10..0xFF19] и продолжается вычисление выражения, то:

    • ch >= 0x0FF21 — всегда false
    • ch <= 0x0FF3A — всегда true
    • ch >= 0x0FF41 — всегда false
    • ch <= 0x0FF5A — всегда true

    Об этом и предупреждает анализатор PVS-Studio.

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

    Чтобы исправить ситуацию, надо добавить дополнительные скобки:

    if (!(((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
          ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
          ((ch >= 0x0FF41) && (ch <= 0x0FF5A))))

    Или переписать условие:

    if (((ch < 0x0FF10) || (ch > 0x0FF19)) &&
        ((ch < 0x0FF21) || (ch > 0x0FF3A)) &&
        ((ch < 0x0FF41) || (ch > 0x0FF5A)))

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

    const bool isLetterOrDigit =    (ch >= 0x0FF10 && ch <= 0x0FF19)  // 0..9
                                 || (ch >= 0x0FF21 && ch <= 0x0FF3A)  // A..Z
                                 || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z
    if (!isLetterOrDigit)

    Обратите внимание, что я убрал часть скобок. Как только что мы видели, большое количество скобок вовсе не помогло избежать ошибки. Скобки должны облегчать чтение кода, а не усложнять. Программисты хорошо помнят, что приоритет сравнений =<, => выше, чем у оператора &&. Поэтому здесь скобки не нужны. А вот если спросить, что приоритетней — && или ||, многие растеряются. Поэтому для задания последовательности вычислений &&, || скобки лучше поставить.

    Почему лучше писать || в начале я описывал в статье "Главный вопрос программирования, рефакторинга и всего такого" (см. главу: Выравнивайте однотипный код «таблицей»).

    Всем спасибо за внимание. Скачивайте и начинайте использовать PVS-Studio. Он поможет выявлять многие ошибки и потенциальные уязвимости на самых ранних этапах.



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers.

    Только зарегистрированные пользователи могут участвовать в опросе. Войдите, пожалуйста.

    Давайте проверим, кто из C и C++ программистов помнит приоритет операторов && и ||
    • 74.3%&& приоритетнее, чем ||328
    • 4.7%|| приоритетнее, чем &&21
    • 20.8%Не уверен, надо подсмотреть в табличку приоритетов операций92

    PVS-Studio

    500,57

    Ищем ошибки в C, C++ и C# на Windows, Linux, macOS

    Поделиться публикацией
    Комментарии 85
      –4
      Примерно поэтому придерживаюсь правила 'лишних скобок не бывает'
        +5
        Вы будто статью не дочитали.
          0
          Лично мне так наглядней. Оператор — явно указанная область действия. Плюс подсветка в IDE и вот это вот все.
          Ошибку в примере нашел сразу.

          Your milage may vary.
          +3
          Простота и читаемость кода лишней не бывает. А множество скобок лишь ухудшают восприятие логики.
            +1

            Вы сейчас просто разом всех лисперов идеологически оскорбили!

            0
            Большое количество скобок увеличивает вероятность ошибки. Лучше разделять операции по строчкам, по идеи для компилятора это не имеет смысла.

            Пример 1
            bool Predicate;
            
            Predicate  = (CONST1 < a) && (CONST2 > a);
            
            Predicate |= (CONST3 < a) && (CONST4 > a);
            
            Predicate |= (CONST5 < a) && (CONST6 > a);
            
            if (false != Predicate)
            {
            ...
            }; 

            и
            Пример 2
            bool PredicateA, PredicateB, PredicateC;
            bool Predicate;
            
            PredicateA = (CONST1 < a) && (CONST2 > a);
            
            PredicateB = (CONST3 < b) && (CONST4 > b);
            
            PredicateC = (CONST5 < c) && (CONST6 > c);
            
            Predicate = PredicateA || PredicateB || PredicateC;
            
            if (false == Predicate)
            {
            ...
            };

            Так же лучше не использовать строки длиннее какого-то значения, у меня не более 100 символов.
              +1
              когда много строк относится к одной операции читать код тоже неприятно. Обычно лучше вынести сложную проверку в функцию, аля:
              inline bool withinSomeRange(int a) {
                  return (a > CONST1 && a < CONST2)
                      && (a > CONST3 && a < CONST4)
                      && (a > CONST5 && a < CONST6);
              }
              
              if (withinSomeRange(a)) {
              // ...
              

              Самое главное в таком подходе то, что условие именуется. С удачно выбранным именем функции во-первых её назначение понятно без чтения реализации, во-вторых проще найти ошибку в условии, нарушающем предполагаемую из имени логику
            +15
            Красивый пример. Коротко, наглядно, понятно. Переписано хорошо. Можно статью джунам показывать в качестве примера.
              +8
              Поэтому мне нравится подсветка скобочек в IDE.
              После этого самого "!" ставишь курсор и смотришь, подсветило в конце или нет.
                +5

                Я вот таким чудом пользуюсь https://marketplace.visualstudio.com/items?itemName=TomasRestrepo.Viasfora там сразу без курсора видно, т.к. для каждой пары задается свой цвет.


                Заголовок спойлера

                image

                  +1
                  А можете вариант из статьи показать как будет выглядеть?
                  ПС: не всегда пишу в студии, так что конкретно студийное решение не всегда хорошо.
                    +1
                    А можете вариант из статьи показать как будет выглядеть?

                    Студии под рукой нет, в VS Code такое же расширение у меня стоит.
                    image

                      +14
                      Красных скобок в итоге много, т.е. первая и последняя в принципе красные, в конкретно этом кейсе не сильно помогает.
                        +9

                        Здесь нужна по настоящему мощная подсветка:

                    0
                    И вариант для тех, кто использует продукты Intellij: plugins.jetbrains.com/plugin/10080-rainbow-brackets
                      +5

                      В Идейке (правда только для Джавы, видимо) и подсветка скобок не нужна, варнинги будут сразу в редакторе при вводе текста. У нас это онлайн-анализатор ловит:



                      Любопытно, кстати, что PVS-Studio анализирует недостижимый код. Мы, например, не выдаём предупреждение, что ch <= 0x0FF3A всегда истинно, потому что оно вообще недостижимо. Видимо, когда PVS-Studio находит всегда ложное условие, оно выдаёт на него варнинг и считает, что этого условия нету и продолжает анализ дальше?

                        +3

                        А ещё нажав Ctrl+Shift+P два раза на выражении, можно увидеть статически выведенный диапазон значений:



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


                        Все эти фичи есть в бесплатной версии IDEA Community :-)

                          +1
                          PVS-Studio иногда анализирует недостижимый код, хотя многие диагностики туда не лезут. Вот именно V560 это в рамках одного выражения этого не учитывает. Так уж диагностика была исторически реализована. Хорошо это или плохо, не знаю. Всё относительно :). Менять поведение особенного смысла не вижу.

                          А вот, например, диагностика V522 ведёт себя иначе. Здесь будет предупреждение:
                          void F(int *a)
                          {
                            if (!a && *a == 2)
                              foo();
                          }

                          V522 CWE-476 Dereferencing of the null pointer 'a' might take place. consoleapplication2017.cpp 125

                          А здесь (в мёртвом коде) уже нет:
                          void F(int *a)
                          {
                            if (a && !a && *a == 2)
                              foo();
                          }

                          Здесь нет V522, зато ожидаемо есть V560 CWE-570 A part of conditional expression is always false: !a. test.cpp 125
                            0

                            Менять не надо, если откровенно путающих ситуаций не возникает, согласен.


                            a && !a && *a == 2

                            А если наоборот, !a && a && *a == 2

                              0
                              V560 CWE-570 A part of conditional expression is always false: a. test.cpp 125
                      +2
                      На самом деле скобочки там как раз почти все избыточны. Приоритет операторов там и так правильный. Не было бы лишних скобочек, было бы очевидно, в чём проблема. Любят насовать скобочек не думая вместо того, чтобы один раз выучить маленькую таблицу приоритетов.
                      +6
                      От этого "!", захотелось избавиться с первого взгляда на код. И только потом спросил себя, а не хотел-ли автор вынести это «НЕ» за общие скобки. Вобщем да, — лишних скобок не бывает.
                        –1
                        А в PHP есть костыль: при использовании ! по-другому интерпретируются приоритеты операторов:

                        if (!$x = 2)

                        Интерпретируется как !($x = 2), а не как (!$x) = 2, как следует из приоритетов.
                          +2
                          Ужас, блин! Тут надо ворнинг пихать, а не другую интрепретацию делать…
                            +1
                            Не надо такое в ворнинг.
                            Это базовый шаблон для PHP
                            if (!$x = foo()) {
                                // функция не вернула not empty результат
                            }
                            // используем $x
                            
                              +1
                              В условиях лучше не использовать присваивания независимо от языка. Лучше писать так:
                              $x = foo();
                              if (!$x) {
                                  // функция не вернула not empty результат
                              }
                              // используем $x
                              

                            +4
                            А ещё в PHP есть операторы && и AND, || и OR, с разными приоритетами (буквенные — ниже)
                              +1

                              !($x = 2) всегда будет false.
                              (!$x) = 2 — тут ошибка.
                              if (!$x = 2) — условие никогда не выполнится же, так как !2 даст false.

                              +10

                              Плохо стало уже при попытке прочитать код. Такое надо сразу переписывать, не дожидаясь диагностики анализатора (всё равно она тут "понятна" как ошибки при компиляции сложносочинённых c++ темплейтов), хотя бы в три строчки, как вы сделали при разборе.

                                +3

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

                                  +6

                                  В данном случае ещё можно, но для большинства случаев я с вами не соглашусь: я в гробу видал идею запоминать приоритеты всех операторов для всех языков, а главное — не буду закладываться, что их назубок помнит тот, кому доведётся читать мой код. Лучше уж лишние скобки, а ещё лучше разбить на подвыражения, как ниже предложил Akon32.

                                    +3

                                    Речь именно что про данный случай. Ошибка то возникла как раз из-за того, что программист запутался в скобках. А было их всего две пары — для if'а и отрицания — проблемы бы вообще не было.
                                    Ну, и никто не заставляет помнить приоритеты всех операторов, но, извините, приоритеты сложения/умножения и ИЛИ/И/НЕ должны отскакивать на зубок, на мой взгляд, и в дополнительных скобках не нуждаться.

                                      +2

                                      У меня в анамнезе Паскаль, в котором приоритет операции and выше, чем операции сравнения. Я, конечно, запомнил разницу, но скобки зачастую ставлю. И не поручусь, что кто-то по запарке не прочитает неправильно.

                                +9
                                Приоритеты & и | запоминаются очень легко. & это умножение, | сложение.
                                  +1

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

                                  +1
                                  А вот если спросить, что приоритетней — && или ||, многие растеряются.

                                  С кем вам приходится работать? Гнать надо этих "многих", извините.


                                  А не лучше ли написать вместо


                                  цитата
                                  const bool isLetterOrDigit =    (ch >= 0x0FF10 && ch <= 0x0FF19)  // 0..9
                                  || (ch >= 0x0FF21 && ch <= 0x0FF3A)  // A..Z
                                  || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z
                                  if (!isLetterOrDigit)


                                  такое:


                                  код
                                  const bool isLetterOrDigit =    (ch >= '0' && ch <= '9')
                                                               || (ch >= 'A' && ch <= 'Z')  
                                                               || (ch >= 'a' && ch <= 'z'); 
                                  if (!isLetterOrDigit)

                                  ?
                                  Ещё я видел code style, где рекомендовалось в особо сложных случаях писать


                                  код
                                  const bool isDownCaseLetter = ...
                                  const bool isUpperCaseLetter = ...
                                  const bool isDigit = ...
                                  if (!(isDigit || isDownCaseLetter || isUpperCaseLetter)) ...
                                    +1
                                    Вот да. Правда, второй вариант не всегда может прокатить, не каждый символ можно так просто добавить в исходник — и в таких случаях я бы рекомендовал завести константы с говорящими именами, типа BIG_LETTER_A или FIRST_BIG_LETTER_EN, а в них уже можно любые коды засовывать, всё равно далее исходник будет читаемым.
                                    А вот третий вариант лично я бы рекомендовал почти всем и всегда. А то и в отдельный метод IsLetterOrDigit.

                                    Ошибку, кстати, нашёл, нарисовав интервалы на прямой, но изначально ожидал от задачи подвох в стиле дефайна скобки в начале файла или чего-то такого.
                                      +5
                                      У вас ASCII символы, а не unicode.
                                      Код '0' будет 0x30 а не как ожидалось 0xff10
                                      unicode-table.com/ru/blocks/halfwidth-and-fullwidth-forms
                                        0

                                        И правда. Тогда нужно u'a', u'z' и т.п., или как там выглядят юникод литералы на С++. Писать 0xff10 — явно плохая практика.

                                          +1
                                          Тогда нужно u'a', u'z'

                                          И получится у вас ещё большая путаница. Эти символы лишь похожи на обычные, но на самом деле другие. Ссылку я не случайно привёл.
                                          a !=a

                                          Вот вы собрались кого-то там гнать с работы, но сами с двух попыток не смогли правильный символ задать.

                                          Может сейчас всё иначе, но раньше использовать исходники в unicode/uft8 — хороший способ познать боль, получив на экране квадратики любимым шрифтом или проблемы с контролем версий.
                                        –4
                                        Еще вариант:
                                        код
                                        const DWORD ranges[RANGE_COUNT][2] = {{0x0FF10, 0x0FF19}, {0x0FF21, 0x0FF3A },
                                        {0x0FF41, 0x0FF5A }};
                                        bool isLetterOrDigit = false;
                                        for (size_t i = 0; i < RANGE_COUNT; ++i){
                                             isLetterOrDigit ||= ch >= ranges[i][0] && ch <= ranges[i][1];
                                        }
                                         

                                        +6
                                        Кстати, вы могли бы этот таск переклассифицировать из баги в фичреквест и не спешить закрывать. Ведь на самом деле ситуация показательная и с другой стороны: лог анализатора не дал трём с половиной программистам понять, что же происходит. Т.е. анализатор умён, спору нет, но мысль свою до людей он не донёс. Может быть, стоит на такие проблемы в логических выражениях выдавать другую ошибку? Какую-нибудь одну, красивую, которая подсветит сереньким цветом выражения, не влияющие на истинность условия, или как-то так? Ведь, насколько я понимаю, вы же всё равно строите деревья разборов этих условий, т.е. самая сложная часть уже есть и отлично работает.
                                          0
                                          Мы по возможности стараемся решать эту задачу. Иногда один общий алгоритм находит ошибку, которая затем для разных случаев разбивается на пяток сообщений для понятности. Но все случаи не предусмотреть.
                                            +1
                                            Ну, тут imho невозможно выдать понятную диагностику, пока не будет понятного кода. Т.е., получается, что надо навязывать codestyle?
                                              +1

                                              Я думаю, будет достаточно добавить в сообщение ", потому что ...". В данном примере,


                                              выражение (ch >= 0x0FF21) && (ch <= 0x0FF3A) всегда false, потому что ch < 0x0FF10 и ch > 0x0FF19, что следует из выражения !((ch >= 0x0FF10) && (ch <= 0x0FF19))

                                              Подчеркнутые места сделать ссылками на соответствующие места в коде. Так как абстрактный вычислитель значений уже есть, то кажется, что это не очень сложно должно быть сделать

                                          –8

                                          Эмм… Это вот вы серьёзно про "многих" которые не знают у чего выше приоритет — && или ||? Они точно программисты? А бухгалтеры многие не знают что приоритетнее — умножение или сложение?

                                            0
                                            Добавил опрос.
                                              +1
                                              Спасибо. Результаты… удивляют.
                                              0

                                              Джва с половиной программиста уже запуталась в казалось бы несложном выражении.

                                                +2
                                                вот только знание приоритетов этих операций нужно лишь для явных нарушений правил «хорошего тона»
                                                  0

                                                  Утверждаете ли вы что запись 2 + 2 * 2 нарушает правила "хорошего тона"? И кто устанавливает эти правила? Чем эта запись лучше/хуже a || b && c?

                                                    +2
                                                    как минимум тем, что не выучив приоритет арифметических операций даже школу не закончишь. Во-вторых, логических операций несколько больше, и, даже если вы способны вспомнить приоритет конъюнкции и дизъюнкции, сомневаюсь, что выражение аля x & y && z | q || w окажется для вас таким же тривиальным.
                                                      +1

                                                      Конечно, нарушает. Найти вам примеры языков, где 2+3*2 будет равно 10?

                                                        –1
                                                        Текст правила или статьи или чего там именно оно «конечно» нарушает в студию. Желательно со ссылкой на место откуда ваше правило взялось.
                                                          –2
                                                          Что докажет ваш пример? Я могу выдумать язык программирования где результатом этого выражения будет «КРАСНЫЙ КРУГ,» и что? Есть языки в которых это вообще не является валидным выражением, и, опять же, что с того?
                                                            0

                                                            Вы же понимаете, что "хороший тон" — это некоторая обобщённая условность. Придерживать за собой дверь — хороший тон, но если это дверь с красным кругом на броне — вход в бомбоубежище, а за вами гонится толпа зомби, то дверь можно уже и не придерживать.


                                                            Также считается хорошим тоном не засовывать взведённый пистолет себе за пояс, чтобы не выстрелить себе в… ногу.


                                                            Возвращаясь от метафор к языкам — если вы можете парой простых приёмов сделать ваш код однозначнее или понятнее для других, то использовать эти приемы — хороший тон.


                                                            В исходном случае правило, что && это умножение, а || — сложение, может быть далеко не очевидно тем, кто привык к другим языкам. И подобная привычка легко может сработать как выстрел в ногу.

                                                              +1
                                                              Я понимаю что «хороший тон», «правила приличия», «мораль» и всё такое штука субъективная и различается от социума к социуму и от индивидуума к индивидууму. Но если вы посмотрите тредик, мои прямые вопросы о том, что собеседники вкладывают в понятие «хорошего тона» применительно к &&/||, успешно игнорируются и мне видимо предлагается догадываться с помощью телепатии.

                                                              В исходном случае правило, что && это умножение, а || — сложение, может быть далеко не очевидно тем, кто привык к другим языкам. И подобная привычка легко может сработать как выстрел в ногу.


                                                              Разверните мысль, какая именно привычка может сработать как выстрел в ногу? Привыкание к языкам в которых || приоритетнее &&? Вполне допускаю что вы правы и лучше бы не привыкать к таким языкам.
                                                          0
                                                          Могу в ответ баянчик приложить:
                                                          image
                                                            0
                                                            Расставленные скобки однозначно показывают какое выражение должно(хотел написать автор) быть записано:
                                                            (2 + 2) * 2 или 2 + (2 * 2).
                                                          +10
                                                          Когда я учился, я знал про приоритет этих операций. Также как и чем отличается сектор диска от кластера и на каком прерывании «висит» клавиатура. Но сегодня мне нужно посмотреть документацию.
                                                          На это есть причины
                                                          • Я отдаю отчет, что в разных языках правила могут быть разными. В статье С, который я не знаю. Вероятно, там такие же правила как в родной Java. Но хороший код работает строго определенным образом, а заказчик не поймет фразы «наверное, код правильный». Я лучше сверюсь с доками или напишу тест на это условие.
                                                          • Я не пишу в таком стиле и переписываю, когда читаю такие конструкции. Вот уже много лет. Мне переписать проще, чем постоянно помнить порядок операндов.
                                                            • Эта строка перегружена, ее стоит упростить. Как уже писали, выделить три boolean const — самое то. В перегруженном коде легче допустить ошибку, статья — наглядный пример.
                                                            • Код будет изменяться, и лучше предотвратить возможность ошибиться. Через полгода кто-то захочет добавить к условию дефисы-запятые и в моих силах сделать так, чтобы он не мог написать неправильно. Даже зеленый джун, писавший на Паскале.

                                                            0
                                                            Да вот самая проблема в том, что Си-подобный синтаксис упёрли многие языки, но приоритет операций лежит выше синтаксиса. Поэтому хитрые выражения типа a?b?c:d:e реально вычисляются по-разному в разных языках.
                                                            Ну и про слово «перегружена» в С++ отдельная «пестня». С одной стороны, там гарантируется short-circuit исполнение операций, поэтому выражение if ( x && x->y ) гарантированно не приведёт к null pointer exception. С другой, это не так, потому что класс x может перегрузить operator&&, а чтобы его вызвать, нужно сначала вычислить оба операнда (левый и правый). Поэтому поведение выражения зависит от типа x.
                                                            Ну и вишенкой на торте — возможность перегрузки оператора «запятая». Вырванное из контекста выражение a = f(x,y) с точки зрения компилятора вообще непонятно что делает.
                                                            Попробуйте разобраться, что выдаст следующая программа:
                                                            // evil.h
                                                            class Evil {
                                                            public:
                                                             double _val;
                                                             Evil(double a) {_val = 1/a;}
                                                             Evil(double a, double b) {_val = a/b;}
                                                             Evil operator, (const Evil rhs) {return Evil(_val,rhs._val);}
                                                             Evil operator, (double b) {return Evil(_val,b);}
                                                            };
                                                            
                                                            double f(Evil a) {return a._val;}
                                                            double f(double a, double b) {return b;}
                                                            
                                                            // main.cpp
                                                            #include <iostream>
                                                            #include <string>
                                                            #include "evil.h"
                                                            
                                                            int main()
                                                            {
                                                              std::cout<<f(1,2);
                                                            }
                                                            
                                                              –1
                                                              поэтому выражение if ( x && x->y ) гарантированно не приведёт к null pointer exception

                                                              К сожалению, не всегда. operator &&, будучи перегруженным, не гарантирует ленивость вычислений аргументов. Если x — некоторый умный указатель, реализующий оператор && и оператор ->, то в потрохах вполне может получиться разыменование нулевого указателя.
                                                            +3
                                                            Несмотря на знание приоритетов, читать выражения типа (a || b && c || d) крайне трудно.
                                                              +1
                                                              Хотя выражение (a || b && c || d) уже неприятно читать, через N коммитов оно может стать
                                                              (a || b && c || d && !e),
                                                              а позже мутировать через
                                                              (a || b && c || d && !e || (f && g))
                                                              в (a || b && c || d && !e || (f && g || !a)).
                                                              Через два года тимлид тратит четыре дня чтобы отловить плавающий баг, доходит до этой строчки, в гневе пишет git history… и видит коммиты от троих людей, двое из которых давно уволились, а init commit писал этот тимлид, когда был молодым.
                                                                +1
                                                                И уже не у кого спросить, зачем появились скобки в хвосте выражения и нельзя по коду понять что хотели достичь последние два коммита, а контакты в телеграмме давно протухли. По старым адресам жили другие люди (тимлид на пару с битой проверял лично).
                                                                Осталось последнее средство, к которому прибегают в крайнем случае. Несмотря на всю его мощь, мало кто рисковал использовать его. Но выбора не было. Тимлид закрылся в своем кабинете, сдул пыль с бумажной стопки и открыл ТЗ…
                                                                конец бесплатного фрагмента
                                                            +1
                                                            А почему бы не сделать выделение метода для условий и назвать его нормально?
                                                            IsDigit, IsUpperLetter, IsLowerLetter?
                                                              +2
                                                              А почему статья на английском не на Хабре?
                                                                –2
                                                                А Delphi/Object Pascal просто не даст скомпилироваться такому коду, пока все скобки у логических операций не проставишь.
                                                                  +2

                                                                  Конкретно эту ошибку система типов Паскаля не ловит.

                                                                  –4
                                                                  Доброго дня! А вы только по c? Есть ли у вас angularJS анализатор? Или может подскажет кто?
                                                                    +2
                                                                    Я далек от мира фронтенда вообще и JavaScript фреймворков в частности. Поэтому вполне серьезно спрашиваю: AngularJS действительно так развился, что для него нужен особенный анализатор, отличный от plain old JavaScript анализатора?
                                                                    Просветите, пожалуйста
                                                                      0

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


                                                                      В IntelliJ IDEA вижу одну инспекцию: Angular|Empty Event Handler. Что бы это ни значило.

                                                                      +1
                                                                      PVS-Studio: C, C++, C#, Java (на подходе).

                                                                      Другие анализаторы: en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis
                                                                        0
                                                                        Хороший список.
                                                                        А для Pascal/Delphi не планируете сделать анализатор? Там в списке про такой язык упоминают в комментариях два универсальных анализатора. :)
                                                                          0
                                                                          Нет. Быть может дальше (но не скоро): JavaScript или PHP.
                                                                      0
                                                                      а не анализировали doom например?
                                                                      +1
                                                                      Я вот сначала подумал, что там word стал отрицательным (0x0FF00 в 16-битном представлении будет отрицательным) и поэтому «больше-меньше» местами поменялось…
                                                                        +3
                                                                        А зачем люди препендят 0 к hex числам? 0x0ff21 вместо 0xff21?
                                                                          +2
                                                                          Действительно странно! Тогда было бы логичнее писать 0x00ff21, чтобы не прочитать в спешке ff с лишним ноликом как 0f f…
                                                                          0
                                                                          Статья хоть и короткая, но как обычно интересная! Занятно то, что даже такой лось, как я, нашел ошибку в коде буквально с первых прочитанных символов. Так что автор кода меня слегка пугает… И, да, я бы предпочел лучшее раскрытие условий в тексте программы (например так, как автор статьи предлагает, хотя варианты в комментариях есть и получше), но при этом скобок я бы не стеснялся. Как выше написали — в гробу я видал правила приоритета в тех или иных языках. При правильном раскрытии условий скобок будет минимум, но ровно там, где нужно.
                                                                            +2
                                                                            Предлагаю пообсуждать такой код, весь в скобочках. Он из проекта, о котором сейчас пишется статья. В нём есть предупреждение PVS-Studio. Какое?)

                                                                            static EHTTP_HeaderParse s_ParseHeader(const char* header, ....)
                                                                            {
                                                                              ....
                                                                              if (sscanf(header, "%u.%u.%u.%u%n", &i1, &i2, &i3, &i4, &n) < 4
                                                                                  ||  sscanf(header + n, "%hu%x%n", &uuu->port, &tkt, &m) < 2
                                                                                  || (header[m += n]  &&  !(header[m] == '$')  &&
                                                                                      !isspace((unsigned char)((header + m)
                                                                                                               [header[m] == '$'])))) {
                                                                                  break/*failed - unreadable connection info*/;
                                                                              }
                                                                              ....
                                                                            }
                                                                              +2

                                                                              Что тут обсуждать, такое переписывать надо.


                                                                              А ошибка, наверно, в том, что второе header[m] == '$' всегда false.


                                                                              !(header[m] == '$') && ... 
                                                                              (header + m)[header[m] == '$']
                                                                                +2
                                                                                Статьи содержат ошибки, иногда FA, но читатели солидарны в одном — переписать надо :D
                                                                              0
                                                                              Я бы для упрощения чтения кода написал так:
                                                                              const bool isLetterOrDigit =    (ch >= 0x0FF10 && ch <= 0x0FF19)  // 0..9
                                                                              || (ch >= 0x0FF21 && ch <= 0x0FF3A)  // A..Z
                                                                              || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z
                                                                              if (!isLetterOrDigit)

                                                                              Добавлять при переписывании комментарии, вводящие в заблуждение — сомнительная идея.
                                                                              Т.к. (ch >= 0x0FF10 && ch <= 0x0FF19) это не 0..9, а FULLLWIDTH DIGIT 0 .. FULLLWIDTH DIGIT 9, аналогично с остальными диапазонами.

                                                                              Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                                                                              Самое читаемое