Как команда PVS-Studio улучшила код Unreal Engine

[OldFisher]

Поздравляю вашу команду с успешным завершением столь серьёзного и интересного дела.

[agranom555]

Если правильно понял, теперь разработчики из EPIC будут использовать сами ваш анализатор?

[EvgeniyRyzhkov]

Да. Во всяком случае, мы для этого все сделали :-)

[Yeah]

Когда уже Windows проверите, чтобы снять, так сказать, все вопросы?

[Andrey2008]

Надо наверное какую-то акцию придумать. Что-бы достучаться до ЛПР. Но пока в голову ничего не приходит.

[Andrey2008]

Читателям возможно будет интересно познакомиться с дискуссией на Reddit.

[VenomBlood]

Статический анализ конечно помогает, но касательно некоторого:

if (Token.TokenType == FBasicToken::TOKEN_Identifier ||
FBasicToken::TOKEN_Guid) //<==

Разве подобные вещи не обязаны генерировать warning еще на этапе компиляции? Неявный конверт int -> bool, уж тем более в логических операциях, и еще тем более в ленивой их версии — это же практически 100% ошибка.

[Andrey2008]

Подобное очень часто используется на практике и по делу. Пример:

enum Mode { E_RELEASE = 0, E_DEBUG = 123 };

#define IS_DEBUG_ON E_DEBUG

if (Foo() && E_DEBUG)
  WriteLog();

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

[VenomBlood]

А завтра кто-то решит написать E_RELEASE = 1, E_DEBUG = 0 или любую другую комбинацию и все поломается. То что код валидный с точки зрения компилятора я в курсе, я к тому что как-то кривовато это выглядит, и потенциальный источник ошибок.

[Andrey2008]

Но ругаться нельзя. См «ужас-пример» ниже. :)

[AllexIn]

Мне кажется или пример должен выглядеть так:

enum Mode { E_RELEASE = 0, E_DEBUG = 123 };

#define IS_DEBUG_ON E_DEBUG

if (Foo() && IS_DEBUG_ON)
  WriteLog();

И корректнее все же было бы:

if (Foo() && MODE==E_DEBUG)
  WriteLog();

[Andrey2008]

Есть разница. Как должно выглядеть. И как есть на самом деле.

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

Классический пример. Компилятор Visual C++ промолчит на вот это даже с /Wall:

char *p = "string";

Хотя на самом деле надо рекомендовать приписать const:

const char *p = "string";

Но компилятор молчит, ибо уже столько такого понаписали…
А ведь легко можно сделать беду:

void f()
{
  char *p = "string";
  *p = 1;
}

И PVS-Studio знает и проводит более глубокий анализ, учитывая не одну строку, но и как используется указатель. И говорит:
V675 Writing into the read-only memory. consoleapplication1.cpp 123

[Igor_Sib]

А есть подобный анализатор для C#?

[Andrey2008]

Есть (в смысле, вообще в мире): en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis#.NET

А ещё мы морально готовимся начать создавать свой собственный анализатор для C#.

[Igor_Sib]

Пасиба, попробую проверить свой проект.

[EvgeniyRyzhkov]

В будущем — да. В настоящем от нас — пока нет.

[Agent_Smith]

Проверьте пожалуйста Unity. Я думаю у вас получится с ними договориться, особенно после этого поста.

[MorfeusKiev]

У меня такое чувство что в начале проверки PVS-Studio — Unity рухнет, крэшнется Windows, сгорит блок питания и выбъет ближайший трансформатор на массиве :)

[Andrey2008]

«Некрокомментинг». Позволю себе выдержку из заметки "PVS-Studio 7.04". Возможно, кому-то это будет интересно и пригодится.

Анализ Unreal Engine проектов

В плагине PVS-Studio для Visual Studio была добавлена опция AutoloadUnrealEngineLog, включение которой позволяет автоматически загружать отчёт анализатора в окно вывода PVS-Studio после прохождения анализа. Без этой опции загрузку лога необходимо делать вручную через меню плагина.

Также в разделе документации "Проверка Unreal Engine проектов" были описаны изменения стандартных сборочных скриптов, которые позволят проводить сборку и анализ в одно действие. Без модификации скриптов (при добавлении флага -StaticAnalyzer=PVSStudio к аргументам запуска) проводится только анализ проекта, без выполнения его сборки.