Pull to refresh

Comments 52

Расставление комментариев для любого рода анализаторов кода в стиле «тут так и должно быть» — это надругательство над читающими код. В комментариях размещается информация для того, кто этот код читает, а не для роботов. Если робот не способен разобраться — это его проблемы.

Кроме того, добавление в исходный текст свободного проекта комментариев для проприетарной программы — это какой-то нонсенс и перевод его из free в contrib (в терминологии Дебиана).
Расставление комментариев для любого рода анализаторов кода в стиле «тут так и должно быть» — это надругательство над читающими код. В комментариях размещается информация для того, кто этот код читает, а не для роботов. Если робот не способен разобраться — это его проблемы.


Какие есть варианты?
Повторяю: Если робот не способен разобраться — это его проблемы.

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

Я тоже хочу, чтобы робот был всемогущ. Но пока недоросли технологии.
Предлагаю привести 5 примеров ложных срабатываний на хорошем коде (реального приложения). И мы обсудим здесь возможные действия.
С великим удовольствием. Хорошего кода — завались. Осталось установить и запустить анализатор.

apt-get install что?

cppcheck, например. У него тоже есть возможность убирать ложные предупреждения.
Хороший код — который хорошо работает, а не который радует глаз программиста эстетическим совершенством.
Смею не согласиться, так как код пишется программистами и для программистов a = 0; например, а машине сойдет и: xor eax,eax
a = 0 — это не для «хорошего кода», а для «удобного сопровождения».
Собственно, что я и сказал, ведь код сопровождают программисты и чаще всего не те кто его писал
Я не поленился и написал статью о ложных срабатываниях. Готов обсудить разметку кода с помощью комментариев. Как по мне, практически все комментарии вполне по делу.
Работа с ложными срабатываниями в PVS-Studio и CppCat.

Поясните, что Вы хотели сказать. В PVS-Studio присутствует ряд различных механизмов для подавления ложных срабатываний. Не только комментарии.
там вопрос был про то, как прятать предупреждения, заведомо известные пользователю как false positive, на последующих прогонах анализатора без внесения аннотаций в исходники.
суть ссылки сводилась к перечислению некоторых возможных методов (автором из коверити), последний из которых (эвристическое сопоставление новых предупреждений с уже размеченными) даёт весьма достойные результаты, не требуя внесения в исходники.
с pvs и его механизмами подавления не знаком, но механизм аннотаций подразумевает, что подобного там нет.
Если программист ленив и не хочет сделать код более понятным анализатору (а как следствие — он всегда становится понятней и читающему кода), то да — надо использовать комментарий. Как вариант — хитрый хак. Тогда тоже, нужен комментарий. Не вижу проблемы. В 95% то место, куда указал анализатор — пахнет. Не обязательно ошибка, но попахивает. Почти всегда, самое лучшее — устранить причин запаха.
Никакого надругательства не будет, если продублировать комментарий для робота комментарием для человека.

Насчет нонсенса и перевода из free в contrib — не согласен. Ведь компилировать открытый проект проприетарной Студией допустимо? И даже .vcxproj в репозиторий включить можно? Почему же тогда нельзя использовать проприетарный синтаксический анализатор?
А кстати отдельный файл с хинтами для анализатора — оно окей или не окей?
Мне кажется, что вполне нормально, и никакие опенсорсные определения не нарушаются. Просто лежит такой отдельный конфиг для тех, у кого есть ещ1 и вот эта вот приблуда.
Такая разметка в коде — весьма удачный компромисс, используемый почти всеми в индустрии. Плюсы разметки в коде:
  • разметка автоматически попадает в репозиторий и становится доступна всем разработчикам;
  • при модификации кода не надо синхронизировать изменения: Добавили одну строку в начале файла и вместо «не ругаться на строку 15» надо «не ругаться на строку 16»;
  • очень понятный принцип работы.


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

Скажем, пишется нейтральный комментарий вида // this foo bar is intentional, а в файле настроек проекта анализатора указывается, что это соответствует подавлению предупреждений 123 и 456.
Почему бы не использовать прагмы, которые игнорируются компилятором?
С причудами Дебиана, кстати, справиться несложно: комментарии для PVS Studio можно расставить в отдельном бранче и перед проверкой каждый раз обновлять этот бранч из master.
Я бы не назвал этот процесс «несложным». Потому что статический анализатор должен применяться каждым программистом, а не быть встроенным в билд-сервер. То есть надо именно в этом бранче работать, а master обновлять из него, вырезая добавление комментариев из истории.
Собственно, amarao, кажется, что-то напутал. Согласно Debian Policy Manual, p. 2.2.1 (ссылка):
the packages in main must not require or recommend a package outside of main for compilation or execution (thus, the package must not declare a «Pre-Depends», «Depends», «Recommends», «Build-Depends», or «Build-Depends-Indep» relationship on a non-main package)

PVS Studio не нужен ни для компиляции, ни для запуска, так что вроде бы с этим нет проблем. Что я упустил?
Ничего вы не упустили, так и есть. А вот начинать искать обходные пути еще до заглядывания в Debian Policy Manual — явно не стоило)
Безусловно. В своё оправдание скажу, что два часа назад с телефона у меня debian.org не открылся, и я решил покамест поверить amarao на слово :-)
Насчет опечатки №4. Я правильно понял? Эффект Доплера? Как он в браузере-то используется?
Как вы политкорректно называете копипасту опечатками)
Копипаста — причина, опечатка — следствие. Не всякая копипаста дает в результате что-то ошибочное.
Сборка осуществляется с помощью make-файлов. Поэтому просто взять и проверить проект нельзя.

Вы рассматривали вариант передать ваш анализатор в качестве компилятора в конфигуратор, чтобы он сам вызывал уже реальный компилятор?
Что-то вроде:
CXX=<pvs-binary> ./configure
Если да, то какие трудности с этим были?
Трудность тут одна — в файле может и не быть переменной CXX.
Да в принципе примерно так и делается тоже конечно же. Вот здесь описано как в makefile встроить.
Прочитал подробнее про ваш способ «мониторинга» вызовов компилятора — звучит как-то ненадёжно. А если параллельно идёт несколько сборок, как ваша система узнает, какие вызовы нужно отлавливать? Я бы первым делом рассмотрел вариант с модификацией окружения. Я плохо знаю возможности Windows, но скорее всего там, как минимум, можно модифицировать переменную PATH и подставить свои программы, которые потом просто продублируют вызов уже с реальными компиляторами, линкерами и т.д.
Как я понял, это вариант исключительно для однократного ручного применения, для случая, когда надо сначала проверить проект — а потом уже думать, стоит ли вообще использовать статический анализатор. И для такого сценария использования этот вариант выглядит действительно круто.
Все верно, для регулярного использования надо применять другие способы. А для разового — самое оно.

Почему для разового запуска сделан специальный отдельный метод? Чтобы упростить первый запуск, знакомство с инструментом.
У вас уже спрашивали, но как скоро вы собираетесь проверить PVS-Studio при помощи PVS-Studio?
В конце статьи:

Прочитали статью и есть вопрос?
Часто Постоянно к нашим статьям задают одни и те же вопросы. Ответы на них мы собрали здесь: Ответы на вопросы читателей статей про PVS-Studio и CppCat, версия 2014. Пожалуйста, ознакомьтесь со списком.
Я то этот FAQ видел, но конкретно этот вопрос где-то прогледел, сорри.
Этот вопрос звучал в комментариях еще до того, как у PVS появился блог. ЕМНИП, в самой первой статье он тоже был.
Дико извиняюсь, не заметил сразу этот вопрос в списке FAQ. Действительно, как же нелепо.
Опечатки 1 — 2: вот почему нужно использовать switch — меньше кода, встроенный контроль, хотя тоже можно break забыть, правда что бы break не забыть можно в макрос завернуть.

p.s: спасибо за ещё одну отличную статью.
Опечатка 5: вообще не надо присваивать значения внутри условных операторов ибо это усложняет читаемость, а бонусов особых не даёт.
Сравните:
if ((result = foo()) < 0)

result = foo();
if (result < 0)
Да, тут появляется копипаст имени переменной, но обратите внимание насколько проще стал читаться код.

Пишу комментарий только ради того что бы люди задумались над простой мыслью — пишите код так что бы он проще читался и ваши волосы станут мягкими и шелковистыми :)
K switch’у должно прилагаться грамотно проставленные типы. Если в проекте int вместо enum, то у switch’а испаряется «встроенный контроль».
Это гораздо проще контролировать, к тому же за использование int в таких целях обычно дрючат везде, что есть правильно, ну по крайней мере я всегда за такое дрючу, по доброму конечно выражаю кюю и говорю, что хорошо бы переписать более однозначно пока это мало где используется :) да и помимо ограничения набора вариантов с помощью enum, switch будет ругаться даже на двойную проверку одного и тоже значения в int, пусть и предупреждением, но будет.
Sign up to leave a comment.