PVS-Studio впечатлен качеством кода Abbyy NeoML

    image1.png

    На днях компания ABBYY опубликовала исходный код своего фреймворка NeoML. Нам предложили проверить эту библиотеку с помощью PVS-Studio. Это интересный проект с точки зрения анализа, так что мы не стали откладывать его в долгий ящик. Чтение этой статьи не займет у вас много времени, так как проект оказался высокого качества :).

    Исходный код NeoML можно найти на GitHub. Этот фреймворк является кроссплатформенным и предназначен для реализации моделей машинного обучения. Он используется разработчиками компании ABBYY для решения задач компьютерного зрения, обработки естественного языка, в том числе обработки изображений, анализа документов и так далее. В настоящее время поддерживаются такие языки программирования как C++, Java, Objective-C, и скоро к этому списку должен добавиться Python. Основной язык, на котором был написан сам фреймворк, это С++.

    Запуск анализа


    Запустить анализ на этом фреймворке было очень просто. После генерации проекта Visual Studio в CMake я запустила анализ PVS-Studio в Visual Studio на интересующие нас проекты из Solution, исключая сторонние библиотеки. Помимо самого NeoML в решении присутствовали еще такие библиотеки от ABBYY как NeoOnnx и NeoMathEngine. Их я также включила в список проектов, для которых запускался анализ.

    Результаты анализа


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

    Например, в этом проекте очень часто происходит вызов виртуального метода из конструктора. В целом это опасный подход. На такие случаи реагирует диагностика V1053: Calling the 'foo' virtual function in the constructor/destructor may lead to unexpected result at runtime. В общей сложности было выдано 10 таких предупреждений. Подробнее о том, почему это опасная практика и какие проблемы она вызывает можно почитать в этой статье Скотта Майерса "Never Call Virtual Functions during Construction or Destruction". Однако, по всей видимости разработчики понимают, что они делают, и ошибок здесь нет.

    Также есть 11 предупреждений среднего уровня диагностики V803 из раздела "микрооптимизаций". Эта диагностика рекомендует заменить постфиксный инкремент на префиксный, если предыдущее значение итератора не используется. В случае постфиксного инкремента создается лишний временный объект. Конечно же это не является ошибкой, просто маленькая деталь. Если такая диагностика неинтересна, то при использовании анализатора можно её попросту выключить. Ну и в принципе, набор "микро-оптимизаций" и выключен по умолчанию.

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

    Очень часто некоторые диагностики могут быть неприменимы или неинтересны для пользователя и лучше не есть кактус, а потратить немного времени на настройку анализатора. Подробнее о шагах, которые стоит предпринять для того, чтобы сразу подобраться поближе к наиболее интересным срабатываниям анализатора, можно почитать в нашей статье "Как быстро посмотреть интересные предупреждения, которые выдает анализатор PVS-Studio для C и C++ кода?"

    Среди срабатываний из раздела "микрооптимизаций" также есть интересные предупреждения диагностики V802, которая рекомендует расположить поля структуры по убыванию размеров типов, что позволяет снизить размер структуры.

    V802 On 64-bit platform, structure size can be reduced from 24 to 16 bytes by rearranging the fields according to their sizes in decreasing order. HierarchicalClustering.h 31

    struct CParam {
      TDistanceFunc DistanceType; 
      double MaxClustersDistance;
      int MinClustersCount; 
    };

    Всего лишь поменяв местами поле MaxClustersDistance с типом double и перечислитель DistanceType можно снизить размер структуры с 24 до 16 байт.

    struct CParam {
      double MaxClustersDistance;
      int MinClustersCount; 
      TDistanceFunc DistanceType; 
    };

    TDistanceFunc это enum, так что его размер эквивалентен int или меньшему типу, поэтому его стоит перенести в конец структуры.

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

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

    V807 Decreased performance. Consider creating a reference to avoid using the same expression repeatedly. GradientBoostFullTreeBuilder.cpp 469

    image3.png

    Обращение к curLevelStatistics[i]->ThreadStatistics[j] можно заменить на обращение к отдельной переменной. В этой цепочке нет вызова каких-то сложных методов, так что особой оптимизации здесь может и не будет, но код, на мой взгляд, будет читаться куда проще и выглядеть компактнее. К тому же, при поддержке этого кода в дальнейшем будет явно видно, что необходимо обращение именно по этим индексам и ошибки здесь нет. Для наглядности приведу код с заменой на переменную:

    auto threadStatistics = curLevelStatistics[i]->ThreadStatistics[j];
    
    if(threadStatistics.FeatureIndex != NotFound ) {
      if(   threadStatistics.Criterion > criterion
         || ( .... ))
      {
        criterion = threadStatistics.Criterion;
        curLevelStatistics[i]->FeatureIndex    = threadStatistics.FeatureIndex;
        curLevelStatistics[i]->Threshold       = threadStatistics.Threshold;
        curLevelStatistics[i]->LeftStatistics  = threadStatistics.LeftStatistics;
        curLevelStatistics[i]->RightStatistics = threadStatistics.RightStatistics;
      }
    }

    Заключение


    Как видите, с точки зрения статического анализа, кодовая база этого фреймворка оказалось очень чистой.

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

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


    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Victoria Khanieva. PVS-Studio Impressed by the Code Quality of ABBYY NeoML.
    PVS-Studio
    Static Code Analysis for C, C++, C# and Java

    Комментарии 18

      +3
      struct CParam {
      TDistanceFunc DistanceType; 
      double MaxClustersDistance;
      int MinClustersCount; 
      };


      Всего лишь поменяв местами поле MaxClustersDistance с типом double и поле MinClustersCount с типом int можно снизить размер структуры с 24 до 16 байт.
      struct CParam {
      TDistanceFunc DistanceType; 
      int MinClustersCount; 
      double MaxClustersDistance; 
      };

      Наверное правильнее было бы double поставить первым, а TDistanceFunc последним, а то вдруг, TDistanceFunc при он рефакторинге станет другим.


      struct CParam {
        double MaxClustersDistance; 
        int MinClustersCount;   
        TDistanceFunc DistanceType; 
      };

      Помимо этого — еще конечно использование просто int — тоже не хорошо, на некоторых 16 битных системах — int имеет размер 16 бит. Надо тогда использовать int32_t или int64_t.


      По идее эти замечания тоже статические анализаторы тоже должны находить.

        0
        Спасибо большое за замечание, конечно же поле типа double здесь будет самым большим. Поправила пример и объяснения в статье.
          +1

          А еще у меня вопрос по анализатору. По поводу plain int, который тут в структуре указан, может анализатор указать что так делать не надо? Так как это счетчик, то скорее всего тип должен size_t. Ведь NeoML позиционируется как кросплатформенный фреймворк. А вы наверное проверяли сборку под конкретную платформу? Но ведь при переходе на какую-нить другую — уже может быть баг… int превратиться в тыкву 16 битный счетчик. Да к тому же он еще и знаковый, зачем? По-моему анализатор должен был на такое указать… или это не проверяется?

            +1
            PVS-Studio на такое не срабатывает. В общем случае сложно сказать какой конкретно размер был необходим, как и должно ли было поле быть знаковым или нет. Такие общие диагностики весьма вероятно будут очень сильно «шуметь» и быстро станут кандидатами на отключение.
              +2

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

                +1

                Спасибо, понял. Не подумайте, что я придираюсь, я вообще больше по микроконтроллерам, где к структурам и типам вообще требования жесткие, выравнивание и размер, особенно если их сохраняешь в NV памяти, а потом оттуда считываешь. Так вот при переносе кода с одного микроконтроллера на другой, баги из-за типов и их размеров ловили на раз два.
                Но это мы еще пользуемся статическим анализаторами, которые такое отлавливают и сообщают нам о потенциально опасной ситуации сразу — до портирования на другое ядро и то пропускали…
                На самом деле есть даже MISRA C++ Rule 3-9-2, 3-2-1


                The basic numerical types of char, int, short, long, float, double and long double should not be used, but specific-length typedefs should be used. This rule helps to clarify the size of the storage,but does not guarantee portability because of the asymmetric behaviour of integral promotion
                  0
                  This rule helps to clarify the size of the storage,

                  Именно, что «helps». Разве оно там требование? При этом требования к памяти прекрасно рассчитываются для конкретной платформы и с обычными char, short, int, long… Вообще, многие требования и рекомендации MISRA, мягко говоря, весьма спорны.


                  but does not guarantee portability

                  Именно. Очень всё весело становится, когда на целевой платформе не оказывается int8_t, int16_t, int32_t, int64_t — все эти типы необязательны по стандарту (в C точно, для C++ нужно уточнить).


                  На каком-нибудь современном DSP запросто могут быть 32-битные char, short и int и никаких int8_t и int16_t. Да даже ARM до StrongARM не умел загружать/сохранять 16-битные значения (но можно было эмулировать с помощью сдвигов и побайтовых load/store).


                  Тип int такой не просто так — именно это позволило UNIX спокойно работать на 16, 18, 32 и 36-битных машинах уже на заре своего становления.


                  (NOTE: 9, 12, 18, 36-битные слова занимают цело число 8-ричных разрядов.)


                  Так как это счетчик, то скорее всего тип должен size_t.

                  В общем случае — да, но в конкретном случае 2¹⁵ — 1 на 16-битной машине может быть недостижимо большим, как и 2³¹ — 1 на 32-битной и даже на 64-битной. Всё зависит от предметной области. (Например, размер квадратной матрицы, хранящей 4-байтные значения.) Так что попытка ругать разработчика приведёт только к справедливому отторжению такого инструмента статического анализа.


                  Итого: нет, наоборот, не стоит использовать типы фиксированной размерности без крайней необходимости. Для индексации потенциально «бесконечных» множеств — size_t, для расчётов ≥ int, для хранения ≥ char (если char как целое, то обязательно с указанием signed/unsigned).

                    0
                    Именно. Очень всё весело становится, когда на целевой платформе не оказывается int8_t, int16_t, int32_t, int64_t — все эти типы необязательны по стандарту (в C точно, для C++ нужно уточнить).

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


                    Т.е. в случае если вы просто int указали и используете его для всего диапазона значений для 32 битного int, а потом перешли на платформу, где int это 16 bit, то там повалятся ошибки.


                    Ну тогда надо явно указать int_least32_t — теперь везде будет 32 бита или больше. Но вы уже знаете, что ваше значение не должно перевалить за 32 бита.

                      +1
                      Т.е. в случае если вы просто int указали и используете его для всего диапазона значений для 32 битного int

                      То вы совершили ошибку, очевидно же. Не нужно так делать. (Если мы про Си, а не про POSIX.) У стандартных типов есть минимальные размеры:


                      • char — 8 бит;
                      • short — 16 бит;
                      • int — 16 бит;
                      • long — 32 бита;
                      • long long — 64 бита;
                      • float — 32 бита;
                      • double — 64 бита;
                      • long double — 64 бита.

                      и граничные значения:


                      • signed char — от -127 до +127;
                      • short — от -16383 до +16383;
                      • и т.д.

                      Заметьте, что не -128 и не -16384, так знак может быть представлен разными способами.


                      Ну тогда надо явно указать int_least32_t — теперь везде будет 32 бита или больше.

                      Если это вам нужно, то да. В текущих реалиях единственная причина использовать least типы как раз в использовании int_least32_t вместо long, чтобы на 64-bit SysV ABI сэкономить место. (И тут нужно ещё подумать, а нужно ли экономить.)


                      int_least8_t и int_fast8_t

                      Вот в случае ARM до StrongARM одним из способов для int_least16_t и int_fast16_t будет выделение для них 16 и 32-битных ячеек: в первом случае медленно побайтово загружаем/сохраняем и собираем/разделяем сдвигами и экономим память, во втором просто используем машинное слово и ценой памяти получаем быстрые операции.


                      Ну и дополнение: всё ещё зависит от целевой платформы, для которой пишется код. Это может быть C89 (да, не все компиляторы до сих пор умеют C99), C99, C11 (C++98, C++03, C++11, C++14, C++17 — что там ещё есть?). А может быть POSIX, который не поддерживает 16-битные платформы в принципе — для него ограничения на обсуждаемые типы такое же, как в Си, за исключением, что int не меньше 32 бит. (То есть UNIX V7 на PDP11 и XENIX на 286 никогда не сделать POSIX системами, не сломав ABI.)


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


                      P.P.S. В общем, не имеет смысла использовать int_least8_t — он не может быть меньше char никогда (sizeof char = 1, по определению), и вместо него всегда использовать signed char (char может быть как signed, так и unsigned по умолчанию — артефакт долгого развития языка до стандартизации). Аналогично с 16-битными типами: все эти least-типы для 8 и 16 просто для унификации существуют. А fast 8 и 16 практически всегда можно заменить на int в современных реалиях.

                        0
                        char — 8 бит;
                        short — 16 бит;
                        int — 16 бит;
                        long — 32 бита;
                        long long — 64 бита;
                        float — 32 бита;
                        double — 64 бита;
                        long double — 64 бита.

                        Я не знаю, про Си ли мы, в данном случае код написан на С++. Так вот там точно не так. Думаю, что и в Си тоже не так.


                        char — это модель байта в данной системе. Чисто теоретически на 4-битной машине или 7-битной байт может быть и 4 бита и 7 бит, так же как он может быть размером 64 бита или 9 бит (например, на PDP10).


                        Но в С++14 стандарте сделали небольшое ограничение и сказали, мол что char должен быть такого размера, чтобы вмещать 8-битный UTF8 символ. Поэтому таки да, на С++14 минимальный размер char это 8 бит. И кстати sizeof возращает размер не в байтах, в традиционном представлении, а в количестве char. Т.е. если чар равен 9 битам, то sizeof(char) == 1, а какой-нибудь sizeof(int) == 2 == 18 битам.


                        По минимальным размерам в битах для целых в принципе согласен, так как размер остальных типов определяются неравенствами типа:
                        1 == sizeof(char) <= sizeof(short) <= sizeof(int) <= sizeof(long) <= sizeof(long long)


                        А вот для float и double — тут вообще ничего не определено. Поэтому в микроконтроллерах float может и 24 бита — PIC16 и PIC18 компиляторы не дадут соврать. А double поголовно 32 бита по умолчанию на том же IAR компиляторе под все 8 битные и 16 битные микроконтроллеры.


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


                        Поэтому это хорошее правило от МИСРЫ и полезное, оно не просто так было сделано.


                        И еще одно удобство, что если ваша система не поддерживает int32_t или int8_t, например, то как миниму код не должен скомпилироваться.


                        А так вы взяли int, предполагаю, что он у вас 32 бит, но на одной машине он 16 бит, на другой 32, ваш код соберется в любом случае, но работать не будет — это не правильно.

                          +1
                          Так вот там точно не так.

                          Это голословное утверждение.


                          Думаю, что и в Си тоже не так.

                          А вы не думайте, а проверте, если не верите:


                          C99 5.2.4.2.1 Sizes of integer types <limits.h>

                          Для char — сказано напрямую, для остальных типов вычисляется по лимитам значений. Из разделов 3.5 и 3.6 следует, что биты у нас только и исключительно двоичные.


                          Не верите, что C++ наследует значительный корпус C?


                          C++98 18.2.2 C Library
                          The contents are the same as the Standard C library header <limits.h>.

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


                          Вы неправы.


                          char — это модель байта в данной системе. Чисто теоретически на 4-битной машине или 7-битной байт может быть и 4 бита и 7 бит

                          Не может, CHAR_BIT по стандарту не меньше 8 бит. И это зафиксировано в первом стандарте C89, никуда не делось и в последующих и перекочевало в С++.


                          И кстати sizeof возращает размер не в байтах, в традиционном представлении, а в количестве char.

                          Неправда, sizeof возвращает размер в байтах:


                          C99 3.6 byte — addressable unit of data storage large enough to hold any member of the basic character set of the execution environment
                          C99 3.7.1 single-byte character — bit representation that fits in a byte
                          C99 6.5.3.4 The sizeof operator — The sizeof operator yields the size (in bytes) of its operand...

                          — и в Си


                          C++98: 1.7 The C++ memory model
                          The fundamental storage unit in the C++ memory model is the byte. A byte is at least large enough to contain any member of the basic execution character set and is composed of a contiguous sequence of bits, the number of which is implementation-defined. The least significant bit is called the low-order bit; the most significant bit is called the high-order bit. The memory available to a C++ program consists of one or more sequences of contiguous bytes. Every byte has a unique address.
                          5.3.3 Sizeof
                          The sizeof operator yields the number of bytes in the object representation of its operand.

                          ­— и в Си с двумя плюсами.


                          то sizeof(char) == 1

                          А это я написал в комментарии, на который вы отвечаете.


                          А вот для float и double — тут вообще ничего не определено.

                          Как же не определено:


                          C99 5.2.4.2.2 Characteristics of floating types <float.h>
                          The values given in the following list shall be replaced by constant expressions with implementation-defined values that are greater or equal in magnitude (absolute value) to those shown, with the same sign:
                          FLT_DIG 6
                          DBL_DIG 10
                          LDBL_DIG 10

                          — Да, не говорят, в каком точно формате должно храниться, но что мантисса для float должна вмещать не менее 6 десятичных цифр — говорят. Там же сказано, что десятичная экспонента должна вмещать значения от -37 до +37.


                          При двоичном представлении для мантиссы из 6 десятичных цифр нужно как минимум 20 бит, для 10 — 34; для экспоненты — 8 бит. Плюс бит на знак. Итого минимум 29 бит на float при двоичном представлении, 43 для double и long double. При BCD формате требования будут выше.


                          Так что да, не 32 и 64, но и вполне определена минимальная точность. Это я спутал с другой частью:


                          C99 Annex F (normative) IEC 60559 floating-point arithmetic

                          — Вот тут уже полное соответствие IEEE 754, IEEE 854. Будем теперь знать, что это не требование...


                          Поэтому в микроконтроллерах float может и 24 бита — PIC16 и PIC18 компиляторы не дадут соврать. А double поголовно 32 бита по умолчанию на том же IAR компиляторе под все 8 битные и 16 битные микроконтроллеры.

                          И, как видно по цитатам из стандарта, это не Си. И если вы вынуждены разрабатывать под такую платформу, то в первую очередь нужно выяснить отличия от стандартного Си. (Для PIC16, вот, я вижу с ходу AN575 «IEEE 754 Compliant Floating Point Routines», где «Routines for the PIC16/17 families are provided in a modified IEEE 754 32-bit format together with versions in 24-bit reduced format.»)


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

                          Ну дак нужно для начала первые главы стандарта прочитать, чтобы понимать, на каком языке писать. А потом «особенности» целевой платформы изучить. (Для платформ с кривым компилятором все эти char/short/int/long заменяются sed'ом на специфические типы в одну команду. Но м)


                          Поэтому это хорошее правило от МИСРЫ и полезное, оно не просто так было сделано.

                          Ограниченно полезное. Оно помогает разработчикам, которые неспособны прочитать документацию на инструмент, которым работают. Ну, если пытаться масштабировать разработку человеко-часами и для ускорения разработки нанимать больше народа… тогда да, это полезное правило, чтобы не угробить весь проект.


                          Ну а если у нас не сырые джуниоры, то это плохое правило, ломающее переносимость кода, написанного на Си. На настоящем Си.


                          MISRA a top coding standard for embedded industries, including automotive

                          Не нужно натагивать правила из embedded (даже его части) на весь остальной мир. Ну а правила, диктующие, что при ложном срабатывании нужно переделывать корректный код, чтобы удовлетворить кривой чекер...


                          Да, там есть интересные правила, из которых есть что позаимствовать (но, опять же, в зависимости от целевой области — разные множества).


                          И еще одно удобство, что если ваша система не поддерживает int32_t или int8_t, например, то как миниму код не должен скомпилироваться.

                          Отличное удобство. Для проекта на несколько файлов — может быть. А для проекта на 30К файлов это так себе. Гораздо удобнее, когда всё это хозяйство просто компилируется на новой платформе стандартным Си и работает.


                          А так вы взяли int, предполагаю, что он у вас 32 бит, но на одной машине он 16 бит,

                          Вот только не надо снова мне приписывать ваших ошибок. Я уже на это отвечал.


                          Дополнение: Про то, что в int влезет как минимум 16 бит я не забуду никогда (слишком много раз перечитывал стандарт, видимо), да и опыт немалый был работы на Си на 8-ми и 16-битных платформах. А вот про то, что на POSIX платформе в int влезет 32 — постоянно забываю (благо один POSIX.1 больше C99 в двадцать раз).


                          P.S. Если мы говорим Си без дополнения, то с 90-го года подразумеваем ISO/IEC 9899. Если мы говорим Си++, то с 98-го года подразумеваем ISO/IEC 14882. О странных отклонениях я ничего в комментариях выше не писал.

                            0

                            Я не знаю так хорошо Си как вы, а Си++ это другой язык. Поэтому


                            Это голословное утверждение.

                            Нет это утверждение стандарта С++


                            6.9.1 Fundamental types [basic.fundamental]
                            1 Objects declared as characters (char) shall be large enough to store any member of the implementation’s basic character set.

                            Ничего про то, что такое базовый набор символов в стандарте нет. Зато я точно знаю, что есть Packed ASCII — размер которого 6 бит. Ничто не запрещает принять его за стандартный набор символов для какой-нибудь системы.


                            Но


                            Неправда, sizeof возвращает размер в байтах:

                            Тут такое дело, да sizeof возвращает в байтах, но до С++14, байт был определен как:


                            The fundamental storage unit in the C++ memory model is the byte. A byte is at least large enough to contain any member of the basic execution character set.

                            Как я уже сказал, что такое набор символов уточнено не было, поэтому 6 битный Packed ASCII вполне себе удовлетворял этому требованию. И по сути определение byte совпадало с определение char. Но в С++14 добавили


                            and the eight-bit code units of the Unicode UTF-8 encoding form and is composed of a contiguous sequence of bits, the number of which is implementationdefined.

                            Что как бы указывает, что минимально байт может быть только 8 бит… но вот char, так и остался минимальным набором символов. В любом случае для 9 битного char sizeof(char) вернет 1, что будет говорить, что байт равен 9 битам, точно также как и для 64 битного char, sizeof(char) тоже вернет 1, в таком случае байт будет равен 64 битам. Поэтому логично сказать, что sizeof возвращает размер в символах, а не в байтах, так как обычно под байтом в программировании (а не стандартом С++) подразумевается 8 бит, а символ может быть практически любого размера.


                            There are five standard signed integer types: “signed char”, “short int”, “int”, “long int”, and “long long int”. In this list, each type provides at least as much storage as those preceding it in the list. There may also be implementation-defined extended signed integer types. The standard and extended signed integer types are collectively called signed integer types. Plain ints have the natural size suggested by the architecture of the execution environment;

                            Ограничение стоит только на то, что арифметика int должна подчиняться степени двойки, т.е. для 3-ного компьютера — все равно размер int должен быть характеризоваться степенью двойки


                            4 Unsigned integers shall obey the laws of arithmetic modulo 2n where n is the number of bits in the value representation of that particular size of integer.

                            Про float.


                            Да, не говорят, в каком точно формате должно храниться:

                            Верно не говорят, в вашем примере вы взяли IEEE 754, а уверены, что других нет, например — posit, bfloat16 или minifloat.


                            Опять же, я говорю про С++, а вы про Си, я не знаю, что там в Си, в С++ (совершенно другом языке) все не так. Все определяется реализацией под конкретную платформу, и double вполне себе может быть размером как float:


                            8 There are three floating-point types: float, double, and long double. The type double provides at least as much precision as float, and the type long double provides at least as much precision as double. The set of values of the type float is a subset of the set of values of the type double; the set of values of the type double is a subset of the set of values of the type long double. The value representation of floating-point types is implementation-defined. [ Note: This International Standard imposes no requirements on the accuracy of floating-point operations; see also 21.3. — end note ] Integral and floating types are collectively called arithmetic types. Specializations of the standard library template std::numeric_limits (21.3) shall specify the maximum and minimum values of each arithmetic type for an implementation.

                            Единственно, что сказано, что максимальное и минимальное значение должно быть определено в стандартной библиотеке под конкретную реализацию.


                            Да, там есть интересные правила, из которых есть что позаимствовать (но, опять же, в зависимости от целевой области — разные множества).

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


                            Отличное удобство. Для проекта на несколько файлов — может быть. А для проекта на 30К файлов это так себе. Гораздо удобнее, когда всё это хозяйство просто компилируется на новой платформе стандартным Си и работает.

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


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


                            Я говорю, про утвержденный С++17 стандарт, а вы про Си. Как то мы спорим о теплом и мягком. Но все равно спасибо за диалог… а стандарт давно не лазил :)

                              0
                              поэтому 6 битный Packed ASCII вполне себе удовлетворял этому требованию

                              Нет, не удовлетворял и не мог удовлетворять в принципе. Сколько раз ещё повторить про CHAR_BIT и его минимальный размер? И я привёл ссылку из C++98. Слишком старо? Боитесь несовместимых изменений? Вот C++17:


                              21.3.5 Header synopsis
                              The header defines all macros the same as the C standard library header <limits.h>.

                              — Никуда декларация совместимости с Си не делась. Даже более того, добавили:


                              See also: ISO C 5.2.4.2.1

                              — Прямая отсылка в стандарт Си. И я этот раздел даже цитировал в предыдущем сообщении!


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

                              Сочувствую тем, кто считает байт 8-битным, а ещё больше их окружению: они опасны в своём заблуждении.


                              Байт — это упорядоченный набор бит, минимально адресуемая ячейка. Без какого либо ассоциированного типа! А char — это символ (а по совместительству ещё и целое число). И по обоим стандартам символ занимает одну ячейку хранения, строго. Причём строго то только по историческим причинам: «нет языка кроме английского».


                              Не нахожу логики в измерениях места, занимаемыми объектами, ничего общего не имеющего с символами в символах.


                              Да и чему эта демагогия? Вы утверждали


                              И кстати sizeof возращает размер не в байтах

                              Что я опроверг цитатами из стандартов: и для Си, и для Си++. Стандарты чётко определяют, что в байтах. И ничего придумывать больше не надо.


                              Specializations of the standard library template std::numeric_limits (21.3) shall specify the maximum and minimum values of each arithmetic type for an implementation.
                              Единственно, что сказано, что максимальное и минимальное значение должно быть определено в стандартной библиотеке под конкретную реализацию.

                              Что ж вы снова не дочитали секцию 21.3, отсылку к которой приводит ваша же цитата? И там не только для целых, но для типов с плавающей запятой, вот прямо в самом конце:


                              The header defines all macros the same as the C standard library header <float. h>.
                              See also: ISO C 5.2.4.2.2

                              И я этот раздел, опять же, даже цитировал в предыдущем сообщении!


                              Ещё раз: Си++ родился как надстройка над Си (изначально это был препроцессор), но со временем из-за разногласий групп стандартизации, языки разошлись немного. Тем не менее, Си++ поддерживает бинарную совместимость с Си: вот в виде этих вот отсылок к лимитам базовых типов и via extern "C". Без этого Си++ не смог бы использовать огромный корпус уже написанного кода на Си.


                              например — posit, bfloat16 или minifloat.

                              Типы есть, но если они не влезают в минимальные указанные стандартом границы, то они не могут быть соответствующим типом в стандартном Си. Вот хоть ты тресни, но не влезут 16 и 24 битные float в необходимый минимум.


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


                              P.S. Даже Markdown парсер на Хабре устал и сломался .)

                        0

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


                        Так, я для криптографии (внутри, но не на уровне API) их использую и необходимость переноса на гипотетическую 36-битную платформу потребует дополнительной работы пересмотреть код, но пока этого не случилось, можно упростить себе жизнь.


                        К таким ограниченным платформам относится часто и системное программирование уровня ядра ОС: если в ОС принято использовать u8, u16, u32, u64, то нужно их и использовать (но только для того, для чего они предназначены).

                  +3
                  Такие диагностики плохи, так как они дают много бессмысленного шума. В С и C++ потенциально опасно всё :). Не ругаться же теперь на каждую строчку. Мы стараемся ругаться не на то, что теоретически может быть опасным, а то, что опасно вполне себе в реальности. Поэтому рассматриваются более частные случаи. К данной теме по смыслу близки:
                  1. V1026. The variable is incremented in the loop. Undefined behavior will occur in case of signed integer overflow.
                  2. V1028. Possible overflow. Consider casting operands, not the result.
                  3. V1029. Numeric Truncation Error. Return value of function is written to N-bit variable.
                  4. 64-битные диагностики
                  5. И т.д.

                  P.S. Да, как Вы заметили ниже, есть MISRA. Но это отдельная история :).

                    0

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

              +11

              Приятно слышать! Спасибо)
              В NeoML статический анализ не использовался. Как раз думали попробовать PVS-Studio))

                +2
                Всегда считал, что в дисциплинированной команде с code review и без анализатора хороший код.
                P.S. Это не отменяет того факта, что PVS продукт хороший, нужный.

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

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