Picture 7

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

Десятое место: «Какая у нас ОС?»


V1040 Possible typo in the spelling of a pre-defined macro name. The '__MINGW32_' macro is similar to '__MINGW32__'. winapi.h 4112

#if !defined(__UNICODE_STRING_DEFINED) && defined(__MINGW32_)
#define __UNICODE_STRING_DEFINED
#endif

Здесь была допущена опечатка в имени макроса __MINGW32_ (MINGW32 объявляет __MINGW32__). В других местах проекта как раз проверка написана правильно:

Picture 3


Это, кстати, была не только первая ошибка в статье "CMake: тот случай, когда проекту непростительно качество его кода", но и вообще первая реальная ошибка, найденная диагностикой V1040 в реальном открытом проекте (19 августа 2019).

Девятое место: «Кто первый?»


V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. mir_parser.cpp 884

enum Opcode : uint8 {
  kOpUndef,
  ....
  OP_intrinsiccall,
  OP_intrinsiccallassigned,
  ....
  kOpLast,
};

bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) {
  Opcode o = !isAssigned ? (....)
                         : (....);
  auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....);
  lexer.NextToken();
  if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {
    intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind()));
  } else {
    intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....));
  }
  ....
}

Нам интересна следующая часть этого кода:

if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {
  ....
}

Оператор '==' имеет более высокий приоритет, чем тернарный оператор (?:). Из-за этого условное выражение вычисляется неправильно. Написанный код эквивалентен следующему:

if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) {
  ....
}

А с учётом того, что константы OP_intrinsiccall и OP_intrinsiccallassigned имеют ненулевые значения, то это условие всегда возвращает истинное значение. Тело ветки else является недостижимым кодом.

Эта ошибка пришла в наш топ из статьи "Проверка кода компилятора Ark Compiler, недавно открытого компанией Huawei".

Восьмое место: «Опасность побитовых операций»


V1046 Unsafe usage of the bool' and 'int' types together in the operation '&='. GSLMultiRootFinder.h 175

int AddFunction(const ROOT::Math::IMultiGenFunction & func) {
  ROOT::Math::IMultiGenFunction * f = func.Clone();
  if (!f) return 0;
  fFunctions.push_back(f);
  return fFunctions.size();
}

template<class FuncIterator>
bool SetFunctionList( FuncIterator begin, FuncIterator end) {
  bool ret = true;
  for (FuncIterator itr = begin; itr != end; ++itr) {
    const ROOT::Math::IMultiGenFunction * f = *itr;
    ret &= AddFunction(*f);
  }
  return ret;
}

Исходя из кода ожидалось, что функция SetFunctionList обходит список итераторов. И если хоть один из них будет невалидным, то возвращаемое значение будет false, иначе true.

Однако в реальности функция SetFunctionList может возвращать значение false даже для валидных итераторов. Разберёмся в ситуации.Функция AddFunction возвращает количество валидных итераторов в списке fFunctions. Т.е. при добавлении ненулевых итераторов, размер этого списка будет последовательно увеличиваться: 1, 2, 3, 4 и т.д. Вот тут и начнёт проявлять себя ошибка в коде:

ret &= AddFunction(*f);

Т.к. функция возвращает результат типа int, а не bool, то операция '&=' с чётными числами будет давать значение false. Ведь младший бит чётных чисел всегда будет равен нулю. Следовательно, такая неочевидная ошибка будет портить результат функции SetFunctionsList даже для валидных аргументов.

Если вы внимательно читали код из примера (а вы же внимательно читали, правда?), то вы могли обратить внимание, что это код из проекта ROOT. Мы его, конечно, проверяли: "Анализ кода ROOT — фреймворка для анализа данных научных исследований".

Седьмое место: «Путаница в переменных»


V1001 [CWE-563] The 'Mode' variable is assigned but is not used by the end of the function. SIModeRegister.cpp 48

struct Status {
  unsigned Mask;
  unsigned Mode;

  Status() : Mask(0), Mode(0){};

  Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
    Mode &= Mask;
  };
  ....
};

Очень опасно давать аргументам функций те же самые имена, что и членам класса. Очень легко запутаться. Перед нами как раз такой случай. Это выражение не имеет смысла:

Mode &= Mask;

Меняется аргумент функции. И всё. Этот аргумент больше никак не используется. Скорее всего, надо было написать так:

Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
  this->Mode &= Mask;
};

А это ошибка из LLVM. У нас есть традиция время от времени анализировать этот проект. В этом году у нас также появилась статья о проверке.

Шестое место: «В C++ свои законы»


Следующая ошибка появляется в коде из-за того, что правила С++ не всегда совпадают с математическими правилами или «здравым смыслом». Заметите сами, где в небольшом отрывке кода содержится ошибка?

V709 Suspicious comparison found: 'f0 == f1 == m_fractureBodies.size()'. Remember that 'a == b == c' is not equal to 'a == b && b == c'. btFractureDynamicsWorld.cpp 483

btAlignedObjectArray<btFractureBody*> m_fractureBodies;

void btFractureDynamicsWorld::fractureCallback()
{
  for (int i = 0; i < numManifolds; i++)
  {
    ....
    int f0 = m_fractureBodies.findLinearSearch(....);
    int f1 = m_fractureBodies.findLinearSearch(....);

    if (f0 == f1 == m_fractureBodies.size())
      continue;
    ....
  }
....
}

Казалось бы, условие проверяет, что f0 равно f1 и равно количеству элементов в m_fractureBodies. Похоже, что это сравнение должно было проверить, находятся ли f0 и f1 в конце массива m_fractureBodies, поскольку они содержат найденную методом findLinearSearch() позицию объекта. Однако на самом деле это выражение превращается в проверку, равны ли f0 и f1, а затем в проверку, равно ли m_fractureBodies.size() результату f0 == f1. В итоге, третий операнд здесь сравнивается с 0 или 1.

Красивая ошибка! И, к счастью, достаточно редкая. Пока мы встречали её только в трёх открытых проектах, и, что интересно, все они были как раз игровыми движками. Кстати, это не единственная ошибка, которую мы обнаружили в Bullet. Самые интересные попали в нашу статью "PVS-Studio заглянул в движок Red Dead Redemption — Bullet".

Пятое место: «Что есть конец строки?»


Следующая ошибка легко обнаруживается, если знать об одной тонкости.

V739 EOF should not be compared with a value of the 'char' type. The 'ch' should be of the 'int' type. json.cpp 762

void JsonIn::skip_separator()
{
  signed char ch;
  ....
  if (ch == ',') {
    if( ate_separator ) {
      ....
    }
    ....
  } else if (ch == EOF) {
  ....
}

Это одна из тех ошибок, которые бывает сложно заметить, если не знать, что EOF определен как -1. Соответственно, если пытаться сравнивать его с переменной типа signed char, условие почти всегда оказывается false. Единственное исключение, это если кодом символа будет 0xFF (255). При сравнении такой символ превратится в -1 и условие окажется верным.

В этом топе очень много ошибок, связанных с играми: от движков до открытых игр. Как вы уже могли догадаться, это место также пришло к нам из этой сферы. Больше ошибок вы можете посмотреть в статье "Cataclysm Dark Days Ahead, статический анализ и рогалики".

Четвертое место: «Магия числа Пи»


V624 There is probably a misprint in '3.141592538' constant. Consider using the M_PI constant from <math.h>. PhysicsClientC_API.cpp 4109

B3_SHARED_API void b3ComputeProjectionMatrixFOV(float fov, ....)
{
  float yScale = 1.0 / tan((3.141592538 / 180.0) * fov / 2);
  ....
}


Небольшая опечатка в числе Пи (3,141592653...), пропущено число «6» на 7-ой позиции в дробной части.

Picture 4
Возможно, ошибка в десятимиллионной доле после запятой и не приведет к ощутимым последствиям, но все-таки стоит пользоваться уже существующими библиотечными константами без опечаток. Для числа Пи, например, есть константа M_PI из заголовка math.h.

Эта ошибка из уже знакомой нам по шестому месту статьи "PVS-Studio заглянул в движок Red Dead Redemption — Bullet". Если вы ещё не отложили её на потом, то это последний шанс.

Небольшое лирическое отступление


Вот мы и приблизились к тройке самых интересных ошибок. Как вы могли заметить, они отсортированы не по катастрофичности последствий их наличия, а по сложности обнаружения. Ведь в конечном счёте самое главное преимущество статического анализа над code review в том, что машина не устаёт и ничего не забывает. :)

А теперь предлагаю вашему вниманию первую тройку.

Picture 8


Третье место: «Неуловимое исключение»


V702 Classes should always be derived from std::exception (and alike) as 'public' (no keyword was specified, so compiler defaults it to 'private'). CalcManager CalcException.h 4

class CalcException : std::exception
{
public:
  CalcException(HRESULT hr)
  {
    m_hr = hr;
  }
  HRESULT GetException()
  {
    return m_hr;
  }
private:
  HRESULT m_hr;
};

Анализатор обнаружил класс, унаследованный от класса std::exception через модификатор private (модификатор по умолчанию, если ничего не указано). Проблема такого кода заключается в том, что при попытке поймать общее исключение std::exception исключение типа CalcException будет пропущено. Такое поведение возникает потому, что приватное наследование исключает неявное преобразование типов.

Да, не хотелось бы увидеть падение программы из-за упущенного модификатора public. Кстати, уверен, что вы точно хоть раз использовали в своей жизни приложение, исходный код которого мы сейчас смотрели. Это старый добрый стандартный калькулятор Windows, который мы также проверяли.

Второе место: «Незакрытые HTML-теги»


V735 Possibly an incorrect HTML. The "</body>" closing tag was encountered, while the "</div>" tag was expected. book.cpp 127

static QString makeAlgebraLogBaseConversionPage() {
  return
    BEGIN
    INDEX_LINK
    TITLE(Book::tr("Logarithmic Base Conversion"))
    FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a))
    END;
}

Как это часто бывает с C/C++ кодом — из исходника ничего не понятно, поэтому обратимся к препроцессированному коду для этого фрагмента:

Picture 6


Анализатор обнаружил незакрытый тег <div>. В этом файле много фрагментов html-кода и теперь его следует дополнительно проверить разработчикам.

Удивлены, что мы умеем проверять и такое? Когда я впервые это увидел, то был впечатлён. Так что мы немножечко анализируем html-код. Правда, только в коде C++. :)

Это не только второе место, но и второй калькулятор в нашем топе. Со списком всех ошибок вы можете ознакомится в статье "По следам калькуляторов: SpeedCrunch".

Первое место: «Неуловимые стандартные функции»


Вот мы и добрались до первого места. Впечатляюще-странная проблема, которая прошла code review.

Попробуйте обнаружить её сами:

static int
EatWhitespace (FILE * InFile)
  /* ----------------------------------------------------------------------- **
   * Scan past whitespace (see ctype(3C)) and return the first non-whitespace
   * character, or newline, or EOF.
   *
   *  Input:  InFile  - Input source.
   *
   *  Output: The next non-whitespace character in the input stream.
   *
   *  Notes:  Because the config files use a line-oriented grammar, we
   *          explicitly exclude the newline character from the list of
   *          whitespace characters.
   *        - Note that both EOF (-1) and the nul character ('\0') are
   *          considered end-of-file markers.
   *
   * ----------------------------------------------------------------------- **
   */
{
    int c;

    for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile))
        ;
    return (c);
}                               /* EatWhitespace */

Теперь давайте посмотрим, на что ругается анализатор:

V560 A part of conditional expression is always true: ('\n' != c). params.c 136.

Странно, не так ли? Давайте посмотрим на кое-что любопытное в этом же проекте, но в другом файле (charset.h):

#ifdef isspace
#undef isspace
#endif
....
#define isspace(c) ((c)==' ' || (c) == '\t')

Так, а это уже действительно странно… Выходит, если переменная c равняется '\n', то совершенно безобидная на первый взгляд функция isspace( c ) вернёт ложь и вторая часть этой проверки не будет выполнена из-за short-circuit evaluation. Если же isspace( c ) будет выполнена, то переменная c равна или ' ' или '\t', а это явно не равно '\n'.

Конечно, вы можете сказать, что этот макрос похож на #define true false,и такой код никогда не пройдёт code review. Однако этот код прошёл ревью и благополучно ждал нас в репозитории проекта.

Если вам нужен более подробный разбор ошибки, то он есть в статье "Для тех, кто хочет поиграть в детектива: найди ошибку в функции из Midnight Commander".

Заключение


Picture 9


За прошедший год мы нашли много ошибок. Это были привычные ошибки copy-paste, ошибки в константах, незакрытые теги и множество других проблем. Но наш анализатор совершенствуется и учится искать всё больше и больше багов, поэтому это далеко не конец, и новые статьи о проверке проектов будут выходить так же часто, как и раньше.

Если кто-то читает наши статьи впервые, то на всякий случай уточню, что все эти ошибки были найдены с помощью статического анализатора кода PVS-Studio, который мы предлагаем скачать и попробовать. Анализатор умеет выявлять ошибки в коде программ, написанных на языках: C, C++, C# и Java.

Вот мы и дошли до конца! Если же вы пропустили первые два уровня, то предлагаю не упускать возможность и пройти их вместе с нами: C# и Java.



Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Maxim Zvyagintsev. Top 10 Bugs Found in C++ Projects in 2019.

Комментарии (60)


  1. whitemonkey
    19.12.2019 18:41

    Небольшая опечатка в числе Пи (3,141592653...), пропущено число «6» на 7-ой позиции в дробной части.


    Во float (32) вроде 7 значащих цифр.
    const float PI = 3.141592f // Вроде отсутствие ошибочной цифры не вносит особого урона коду?

    Буду рад ответу на вопрос. Есть маленький проект-хобби, пишу софтварную 2d-3d либу (рендер) в образовательных целях.


    1. Andrey2008
      19.12.2019 19:19
      +2

      Вы правы. С практической точки зрения, погрешность мала и наверняка не окажет влияние на что-то. Но ошибка есть ошибка. И если уж совсем быть точным, то программа:

      void main()
      {
        float A = 1.0 / tan((3.141592538 / 180.0) * 5.0f / 2);
        float B = 1.0 / tan((3.1415926538 / 180.0) * 5.0f / 2);
        printf("A=%f\nB=%f\n", A, B);
      }

      Выдаст:
      A=22.903767
      B=22.903765


      Да, последнее десятичное число за пределом погрешности. Но важно само наличие разницы.


      1. mikhaelkh
        19.12.2019 21:01

        Во втором случае тоже ошибка c пропуском цифры константы, pi = 3.14159265358…
        Почему не следуете своему же совету и не пишете M_PI?


        1. Andrey2008
          19.12.2019 21:28

          Согласен :). Ну а так, в данном случае это уже оказалось за пределами точности.

          void main()
          {
            float A = 1.0 / tan((3.141592538 / 180.0) * 5.0f / 2);
            float B = 1.0 / tan((M_PI / 180.0) * 5.0f / 2);
            printf("A=%f\nB=%f\n", A, B);
          }
          

          Выдаёт:
          A=22.903767
          B=22.903765


          1. myxo
            19.12.2019 22:36

            Не совсем понимаю что вы понимаете под «пределами точности». Числа-то разные.

            Другое дело, что whitemonkey, видимо, спрашивал про то, что во флоате этой разницы уже нет в принципе, то есть

            assert(3.1415926f == 3.1415925f); // ok
            assert(3.1415926 == 3.1415925);   // а вот тут assert не пройдет
            

            И тогда ошибка найдена очень правильно, так как хоть результат и пишется в float, а вычисления в tan-то проходят в double.


            1. Andrey2008
              19.12.2019 23:02
              +1

              Дискуссия вышла немного бестолковая :). Дело в том, что в коде в числе PI на самом деле не одна ошибка, а две.

              3.141592538   // В коде.
              3.1415926538  // Я исправил 1 ошибку.
              3.14159265358 // А их там две! Ещё и 5 перед 8 нет.
              

              Я имел в виду, что эта вторая ошибка вообще уже никак не влияет на результат.


        1. GrimMaple
          20.12.2019 13:09

          M_PI не является стандартом, например :)


          1. maksqwe
            23.12.2019 13:53

            в С++20 будет
            www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0631r8.pdf

            например в msvc 2019 16.5 завезут #include


      1. whitemonkey
        19.12.2019 21:27
        +1

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


  1. daiver19
    19.12.2019 20:21

    Я что-то не понял 5-е место. char же знаковый и может быть равен EOF. Неясно, ожидается ли EOF на входе, но почему предлагается использовать int в качесвет фикса?


    1. Andrey2008
      19.12.2019 21:22

      Да. Вот только беда, что и буква 'я' будет представлена как -1 (EOF).


      1. daiver19
        19.12.2019 21:27

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


        1. Andrey2008
          19.12.2019 21:31
          +3

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


          1. MooNDeaR
            19.12.2019 23:57
            -4

            Какой-то выдуманный аргумент. Все программирование — это про соблюдение контрактов. Если в данной точке кода контракт вызываемой функции не подразумевает наличие чего-то произвольного, то этот код более чем валидный.


  1. vanxant
    19.12.2019 20:30
    +2

    Как обычно, передайте респект иллюстратору!


    1. EvgeniyRyzhkov
      19.12.2019 20:51
      +2

      Вот здесь это можно сделать лично.


  1. dzsysop
    19.12.2019 21:50

    Вопрос про пример (2-ое место).
    Если я правильно понимаю фактическая ошибка вызвана вот этой строкой:

    TITLE(Book::tr("Logarithmic Base Conversion"))

    То есть анализатор должен был сообщить о ней либо там где описывается реализация

    TITLE

    либо там где описывается

    Book::tr(

    я что-то упускаю?


    1. Andrey2008
      19.12.2019 22:20

      Если честно, то я вообще не понял вопрос. :(


      1. dzsysop
        19.12.2019 22:49

        Вопрос в том в какой строке из примера ошибка?

        И если она в строке TITLE, почему анализатор нашел ее именно тут, а не при анализе кода в другом месте где описывается функционал этого TITLE?


        1. Andrey2008
          19.12.2019 23:06

          Какой функционал? Это макросы. Они раскрылись внутри функции makeAlgebraLogBaseConversionPage. Соответственно в функции makeAlgebraLogBaseConversionPage и найдена ошибка.


          1. dzsysop
            19.12.2019 23:28

            Я не Сишник.
            Я имею в виду что если ошибка в макросе. То анализатор должен бы ругаться там где декларируется макрос. Нет?

            Можете мне подсказать, что в этом фрагменте кода надо исправить чтобы он стал валидным?


            1. Andrey2008
              19.12.2019 23:45
              +1

              Я имею в виду что если ошибка в макросе. То анализатор должен бы ругаться там где декларируется макрос. Нет?
              Ругаться на содержимое макросов часто невозможно. Пока они не раскроются, неизвестно, верный получится код или нет. Но даже, если не верный, всё равно может быть виноват не сам макрос, а использующий его макрос/код. Или макросу может быть передан неправильный аргумент, на который макрос не рассчитан. Не знаю как просто объяснить.

              В PVS-Studio есть диагностики на тему макросов (пример V1003). Но их мало. В основном работа всегда идёт с уже препроцессированных кодом.

              Можете мне подсказать, что в этом фрагменте кода надо исправить чтобы он стал валидным?
              Нужно исправить один из макросов.


            1. vyo
              19.12.2019 23:50
              +1

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


              Например


              #define MY_MACRO while(somevar) doBlaBla()

              И что теперь? Если somevar и doBlaBla определены в месте вызова, то всё пойдёт как по маслу. А если нет, то возникнет ошибка.
              А ещё если макрос не используется вообще, то в нём хоть синтаксически бред будет — компилятору пофиг. Этакий код Шрёдингера — пока не вызовешь, ошибка и есть, и нет.


              Поэтому в плюсах от макросов и избавляются всеми силами (благо можно). В си с этим хуже.


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


  1. PsyHaSTe
    20.12.2019 00:16

    Спасибо за статью.


    Что любопытно, что в современных языках половина ошибок была бы отловлена на этапе компиляции
    (9 8 6 5 3)


    1. KanuTaH
      20.12.2019 00:53
      +1

      C# — «современный» язык? Сравнение того же char с -1 в нем точно так же не отлавливается на этапе компиляции, насколько я вижу. Точно так же, если сделать переменную int EOF = -1 и сравнить char с ней, компилятор даже не пикнет. Или «современные языки» — это конкретно тот самый, где implicit conversions принципиально запрещены, а остальные недостаточно современны? :)


      1. PsyHaSTe
        20.12.2019 00:58
        +2

        Нет, C# не современный. Современные — Го, Раст, Свифт, Идрис и прочие. Появившиеся в последние 10 лет.


        И даже го конечно пропускает часть из этих проблем, но не все (раз, два, 9 записать невозможно, остальное не проверял).




        А про C#: он хоть и старенький (20 лет это приличный срок), у него есть секретное оружие — рослин и его анализаторы. Примерно как PVS, только если вас бесит какая-то проблема вы можете самостоятельно за пару часов напилить анализ который будет проверяться компилятором, и не придется ждать пока ребята из PVS решат что стоит такую проверку запилить. Буквально: сделали репу, написали что анализировать и должно ли это быть ворнингом/ошибкой, опционально запилили фикс, запушили пакет, добавили себе в проект, нашли все проблемные места. С момента как захотелось зафиксить какую-то фигню до момента как проект для сборки потребовал все такие места починить — буквально час-два.


        1. KanuTaH
          20.12.2019 01:13
          +1

          Ну Идрис, положим, появился таки больше 10 лет назад. Да и Go, скажем так, уже не совсем укладывается :) Но ОК, пусть будет такое деление :)


        1. KanuTaH
          20.12.2019 01:28

          Для C++ тоже есть clang analyzer с его API, к нему можно писать свои плагины для проверки того, что тебе надо ad hoc. Тут где-то не так давно даже статья пробегала про это. Думаю, набив руку, это тоже можно делать довольно быстро.


          1. PsyHaSTe
            20.12.2019 02:30

            А какой процент разработчиков сидит на clang (причем я так понимаю он должен быть довольно свежим?). Мне казалось большинство используют gcc.


            Ну и второй вопрос насколько это удобно. В дотнета весь тулинг на это завязан, потому что language server всё это учитывает когда не только при сборке ошибку выдает, но и подкрашивает ошибочные места, и предлагает исправление. Как с этим дела обстоят у этого analyzer API?


            Но вообще я не знал, что такая штука есть. Это круто.


            1. KanuTaH
              20.12.2019 02:56

              Никто не мешает использовать gcc для сборки продукта, а clang — для анализа в процессе разработки (можно и в дополнение к тому же pvs или coverity). Что касается тулинга — лично у меня это как правило Qt Creator, он хорошо интегрирован с clang, можно запустить анализ прямо из IDE с теми настройками, с которыми хочешь, будет окошечко со списком обнаруженных проблем, из которого можно перейти на соответствующий кусок кода. Quick fix'ы там, по-моему, тоже предлагаются, если они возможны, но я ими ни разу не пользовался. Вот так это примерно выглядит :


              https://doc-snapshots.qt.io/qtcreator-4.0/creator-clang-static-analyzer.html


              1. PsyHaSTe
                20.12.2019 14:15

                Никто не мешает использовать gcc для сборки продукта, а clang — для анализа в процессе разработки

                Мне казалось что как правило гцц проект clang собрать не в силах.


                Что касается тулинга — лично у меня это как правило Qt Creator, он хорошо интегрирован с clang, можно запустить анализ прямо из IDE с теми настройками, с которыми хочешь, будет окошечко со списком обнаруженных проблем, из которого можно перейти на соответствующий кусок кода.

                Не совсем понял, как сделать свою диагностику. На сайте http://clang-analyzer.llvm.org/ говорят "помогайте нам улучшить диагностики, присылайте PR'ы". Но они явно не примут анализ из разряда "Проверка что код для КлиентаА в проекте My.Company.Name.ClientA имеет зависимости от кода для КлиентаБ My.Company.Name.ClientB".


                1. KanuTaH
                  20.12.2019 14:49

                  Мне казалось что как правило гцц проект clang собрать не в силах.

                  Ну это разве что если там используются gcc-специфичные расширения типа нижеупомянутого constexpr интринсика acos(). Но обычно так все-таки стараются не делать, а все-таки стараются писать универсальный код. Кстати, сам факт проверки сборкой на различных компиляторах этому очень способствует.

                  Не совсем понял, как сделать свою диагностику.

                  clang-analyzer.llvm.org/checker_dev_manual.html

                  Вот пример такого плагина-чекера:

                  github.com/Lekensteyn/clang-alloc-free-checker

                  Вот я даже нашел ту недавнюю статью, про которую выше писал:

                  habr.com/ru/company/dsec/blog/473412


                  1. PsyHaSTe
                    20.12.2019 15:40

                    Понял. Любопытно, спасибо. Хотя юзабилити немного страдает.


                    1. KanuTaH
                      20.12.2019 15:57

                      В каком плане?


                      1. PsyHaSTe
                        20.12.2019 17:21

                        Ну типичные задачи:


                        1. как запускать анализаторы a, b, c, d автоматически при билде любого проекта в IDE
                        2. как запускать анализаторы x, y, z только при билде проектов Y и Z
                        3. как хранить настрйки анализаторов в репозитории чтобы при клонировании репы пользователь автоматически конфигурировал локальную сборку с соответствующими анализаторами (аналогично, как .gitattributes конфигурирует локальные настройки гита для конкретного репозитория).


                        1. KanuTaH
                          20.12.2019 17:32

                          Ну это такие уже вопросы… скажем так, рутинного, а не принципиального характера. С той или иной степенью удобства, думаю, их можно решить практически везде, где-то более удобно, где-то менее. Зависит от IDE и системы сборки, их же много разных.


                          1. PsyHaSTe
                            20.12.2019 17:54

                            Ну я так и сказал :)


        1. MrDvorak Автор
          20.12.2019 13:06

          На самом деле пользователи анализатора часто предлагают нам сделать разные диагностики. И обычно мы реализуем их


          1. PsyHaSTe
            20.12.2019 14:14

            Не спорю, но сколько пользователям эту диагностику ждать? Месяц? Два? Полгода? Плюс у вас же свои приоритеты, тогда я свою хотелку не получу никогда.


            С другой стороны вот я в качестве примера: человек попросил от языковой команды определенную фичу, я ему показал, что её самостоятельно можно сделать за час. За час — просто потому, что кода буквально полсотни строк.


            Всё же это совсем разные сроки.


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


            1. Andrey2008
              20.12.2019 15:02

              Не спорю, но сколько пользователям эту диагностику ждать? Месяц? Два? Полгода? Плюс у вас же свои приоритеты, тогда я свою хотелку не получу никогда.
              Становитесь клиентом и получите быстро. :)

              Более того, если хотелки сложные, то намного выгоднее чтобы её делала команда PVS-Studio, а не команда, разрабатывающая проект. Требуется погружение, тестирование, поддержка и т.д. А бизнес любит, когда делают нужный проект, а не развлекаются поделками (пусть и интересными :).


              1. PsyHaSTe
                20.12.2019 15:42

                Более того, если хотелки сложные, то намного выгоднее чтобы её делала команда PVS-Studio

                Обычно не сложные, а простые, но специфические для проекта (поэтому они никогда не попадут в апстрим и кодовую базу всего PVS).


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

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


    1. Groramar
      20.12.2019 15:12
      -1

      Все так. Делфи большую часть того, что PVS ловит, просто не скомпилирует. Нужен плюсам костыль в виде PVS, что поделать.


      1. KanuTaH
        20.12.2019 15:49

        Конечно, не скомпилирует. Ведь в Дельфи банально нет ни тернарного оператора, ни приватного наследования, если уж речь об этой статье. Это все равно, что сравнивать игрушечную машинку с настоящей и говорить «а вот игрушечная не задавит» :)


        1. Groramar
          21.12.2019 23:21

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


          1. KanuTaH
            21.12.2019 23:53

            «Потому, что не нужно» ©? :) Не перегружайте голову, переходите сразу на брейнфак. Там всего 8 операторов на все про все, и больше ничего — остальное «не нужно». Запутаться решительно невозможно.


          1. PsyHaSTe
            22.12.2019 18:28

            В расте точно так же инлайн можно написать, просто это проблем не вызовет:

            if o == if !isAssigned { OP_intrinsiccall } else { OP_intrinsiccallassigned } {
              ....
            }


            Просто потому что приоритеты операторов логичны.

            Хорошая цитата:

            Conclusion: It's a mess. Why does C# do it this way? Because that's how C does it. Why? I give you the words of the late designer of C, Dennis Ritchie:
            In retrospect it would have been better to go ahead and change the precedence of & to higher than ==, but it seemed safer just to split & and && without moving & past an existing operator. (After all, we had several hundred kilobytes of source code, and maybe [three] installations....)

            Ritchie's wry remark illustrates the lesson. To avoid the cost of fixing a few thousand lines of code on a handful of machines, we ended up with this design error repeated in many successor languages that now have a corpus of who-knows-how-many billion lines of code. If you're going to make a backward-compatibility-breaking change, no time is better than now; things will be worse in the future.


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

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


            1. KanuTaH
              23.12.2019 01:11
              +1

              У раста в этом плане своих приколов хватает. Например, с выражениями типа "..1 == var". На первый взгляд может показаться, что это сравнение ренджа со значением переменной var, но на самом деле это RangeTo<bool> и вычисляется как "..(1 == var)". Сюрприз-сюрприз. То же самое с выражениями типа «var == 1..», только там будет RangeFrom<bool>. Как грицца, good operator precedence is impossible, так что его нужно просто знать, вот и все.


              1. PsyHaSTe
                23.12.2019 02:15

                Действительно, довольно неудачная конструкция. Хотя понятно, почему приоритеты так сделаны: чтобы можно было писать a..b+1 без скобочек.


                Спасибо за пример.


  1. Amomum
    20.12.2019 01:06
    +1

    С M_PI есть только одна проблема — это необязательная константа, некоторые компиляторы ее не предоставляют. Поэтому даже в коде STL можно увидеть захардкоженное число пи:


    вроде бы libc++
    template <class _RealType>
    template<class _URNG>
    inline _LIBCPP_INLINE_VISIBILITY
    _RealType
    cauchy_distribution<_RealType>::operator()(_URNG& __g, const param_type& __p)
    {
        uniform_real_distribution<result_type> __gen;
        // purposefully let tan arg get as close to pi/2 as it wants, tan will return a finite
        return __p.a() + __p.b() * _VSTD::tan(3.1415926535897932384626433832795 * __gen(__g));
    }


    1. rogoz
      20.12.2019 01:21

      error: constexpr variable 'pi' must be initialized by a constant expression


      1. Amomum
        20.12.2019 01:23

        Чем компилировали? gcc 9.2 вроде может https://godbolt.org/z/E8OtfX


        1. rogoz
          20.12.2019 01:30

          А clang 9 нет. И судя по всему по стандарту не должно компилироваться.


          1. Amomum
            20.12.2019 01:32

            Ле вздох. Ну, значит просто const.


            1. rogoz
              20.12.2019 02:25

              Вроде как предложение о constexpr <cmath> в С++20 не прошло, печаль.


              1. Amomum
                20.12.2019 02:28

                Но вроде math constants все-таки будут.


        1. mapron
          20.12.2019 01:41
          +1

          Расширение. Я бы не полагался. Передайте -fno-builtin.


  1. Badimagination
    20.12.2019 13:45

    Добрые люди, помогите понять, что означает выражение из 7-го места:
    Status(): Mask(0), Mode(0){};
    Или запрос для поисковика хотя бы дайте.


    1. MrDvorak Автор
      20.12.2019 13:51

      Это список инициализации конструктора по умолчанию


    1. MrDvorak Автор
      20.12.2019 13:53

      Когда в коде будет написано:

      Status tmp;

      То будет вызван этот конструктор, который инициализирует поля Mask и Mode значением 0


  1. dm_frox
    20.12.2019 15:13
    +2

    По поводу Третье место: «Неуловимое исключение». Всегда был уверен, что любые ошибки приватного наследования обнаруживаются на стадии компиляции. А тут такое. Проверил в MSVS. Если поставить в конце catch(…), то catch(std::exception&) молча пропускается, перехват в catch(…). Если убрать catch(…), то в debug возникает abort, в releast ничего. Стало быть полная статическая типизация при перехвате исключения невозможна. Очень полезная информация.