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

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

![0746_Abbyy_NeoML_ru/image1.png](https://import.viva64.com/docx/blog/0746_Abbyy_NeoML_ru/image1.png)

Исходный код [NeoML](https://www.abbyy.com/neoml/) можно найти на [GitHub](https://github.com/neoml-lib)\. Этот фреймворк является кроссплатформенным и предназначен для реализации моделей машинного обучения\. Он используется разработчиками компании ABBYY для решения задач компьютерного зрения, обработки естественного языка, в том числе обработки изображений, анализа документов и так далее\. В настоящее время поддерживаются такие языки программирования как C\+\+, Java, Objective\-C, и скоро к этому списку должен добавиться Python\. Основной язык, на котором был написан сам фреймворк, это С\+\+\.

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

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

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

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

Например, в этом проекте очень часто происходит вызов виртуального метода из конструктора\. В целом это опасный подход\. На такие случаи реагирует диагностика [V1053](https://pvs-studio.ru/ru/docs/warnings/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](https://www.artima.com/cppsource/nevercall.html)"\. Однако, по всей видимости разработчики понимают, что они делают, и ошибок здесь нет\.

Также есть 11 предупреждений среднего уровня диагностики [V803](https://pvs-studio.ru/ru/docs/warnings/v803/) из раздела "микрооптимизаций"\. Эта диагностика рекомендует заменить постфиксный инкремент на префиксный, если предыдущее значение итератора не используется\. В случае постфиксного инкремента создается лишний временный объект\. Конечно же это не является ошибкой, просто маленькая [деталь](https://pvs-studio.ru/ru/blog/posts/cpp/0093/)\. Если такая диагностика неинтересна, то при использовании анализатора можно её попросту выключить\. Ну и в принципе, набор "микро\-оптимизаций" и выключен по умолчанию\.

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

Очень часто некоторые диагностики могут быть неприменимы или неинтересны для пользователя и лучше не есть кактус, а потратить немного времени на настройку анализатора\. Подробнее о шагах, которые стоит предпринять для того, чтобы сразу подобраться поближе к наиболее интересным срабатываниям анализатора, можно почитать в нашей статье "[Как быстро посмотреть интересные предупреждения, которые выдает анализатор PVS\-Studio для C и C\+\+ кода?](https://pvs-studio.ru/ru/blog/posts/cpp/0633/)"

Среди срабатываний из раздела "микрооптимизаций" также есть интересные предупреждения диагностики [V802](https://pvs-studio.ru/ru/docs/warnings/v802/), которая рекомендует расположить поля структуры по убыванию размеров типов, что позволяет снизить размер структуры\. 

[V802](https://pvs-studio.ru/ru/docs/warnings/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

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

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

```cpp

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

_TDistanceFunc_ это _enum_, так что его размер эквивалентен _int_ или меньшему типу, поэтому его стоит перенести в конец структуры\.

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

В целом весь код написан аккуратно и разборчиво, но диагностика [V807](https://pvs-studio.ru/ru/docs/warnings/v807/) указала на пару мест, которые можно сделать чуть более оптимальными и читабельными\. Приведу наиболее наглядный пример:

[V807](https://pvs-studio.ru/ru/docs/warnings/v807/) Decreased performance\. Consider creating a reference to avoid using the same expression repeatedly\. GradientBoostFullTreeBuilder\.cpp 469

![0746_Abbyy_NeoML_ru/image3.png](https://import.viva64.com/docx/blog/0746_Abbyy_NeoML_ru/image3.png)

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

```cpp
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;
  }
}
```

## Заключение

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

Стоит понимать, что один запуск анализа на активно разрабатываемый проект слабо отражает необходимость в статическом анализе, так как многие ошибки, особенно если они были критическими, уже были обнаружены и другими путями, но куда более затратными по времени и ресурсам\. Чуть подробнее этот момент был разобран в статье "[Ошибки, которые не находит статический анализ кода, потому что он не используется](https://pvs-studio.ru/ru/blog/posts/cpp/0639/)"\.

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