Pull to refresh

Comments 20

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.


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

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

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

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

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

Спасибо, понял. Не подумайте, что я придираюсь, я вообще больше по микроконтроллерам, где к структурам и типам вообще требования жесткие, выравнивание и размер, особенно если их сохраняешь в 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
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).

Именно. Очень всё весело становится, когда на целевой платформе не оказывается 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 бита.

Т.е. в случае если вы просто 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 в современных реалиях.

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, ваш код соберется в любом случае, но работать не будет — это не правильно.

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

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


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

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


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. О странных отклонениях я ничего в комментариях выше не писал.

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


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

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


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 стандарт, а вы про Си. Как то мы спорим о теплом и мягком. Но все равно спасибо за диалог… а стандарт давно не лазил :)

поэтому 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 парсер на Хабре устал и сломался .)

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


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


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

Такие диагностики плохи, так как они дают много бессмысленного шума. В С и 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. Но это отдельная история :).

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

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

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

auto threadStatistics = curLevelStatistics[i]->ThreadStatistics[j];

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

Вот вариант, эквивалентный коду до изменений, дополнительно защищающий от случайного присвоения «не туда» и/или «не тому»:

auto const &threadStatistics = curLevelStatistics[i]->ThreadStatistics[j];

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

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

Вариант из статьи:

mov rax, QWORD PTR -152[rbp]
lea rdx, 64[rax]
mov eax, DWORD PTR -20[rbp]
mov esi, eax
mov rdi, rdx
call FObj::CArray<NeoML::CGradientBoostNodeStatistics*, FObj::CurrentMemoryManager>::operator[](int)
mov rax, QWORD PTR [rax]
lea rdx, 40[rax]
mov eax, DWORD PTR -28[rbp]
mov esi, eax
mov rdi, rdx
call FObj::CArray<NeoML::CThreadStatistics, FObj::CurrentMemoryManager>::operator[](int)
mov rdx, rax
lea rax, -144[rbp]
mov rsi, rdx
mov rdi, rax
call NeoML::CThreadStatistics::CThreadStatistics(NeoML::CThreadStatistics const&)

Предлагаемый вариант:

mov rax, QWORD PTR -56[rbp]
lea rdx, 64[rax]
mov eax, DWORD PTR -20[rbp]
mov esi, eax
mov rdi, rdx
call FObj::CArray<NeoML::CGradientBoostNodeStatistics*, FObj::CurrentMemoryManager>::operator[](int)
mov rax, QWORD PTR [rax]
lea rdx, 40[rax]
mov eax, DWORD PTR -28[rbp]
mov esi, eax
mov rdi, rdx
call FObj::CArray<NeoML::CThreadStatistics, FObj::CurrentMemoryManager>::operator[](int)
mov QWORD PTR -40[rbp], rax

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

Т.к. «auto threadStatistics = curLevelStatistics[i]->ThreadStatistics[j];» копирует всю структуру в которой может быть 100500 толстых полей, которые являются !POD типами и их копирование приведёт к выделению памяти которая в последующем коде не используется.

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

ИМХО, в данном месте компилятор справится самостоятельно, сохранив адрес curLevelStatistics[i]->ThreadStatistics[j] и переиспользуя его в дальнейших сравнениях.
Sign up to leave a comment.