How to become an author
Search
Write a publication
Pull to refresh

Comments 52

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

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


Какие есть варианты?
Total votes 18: ↑15 and ↓3+12
Повторяю: Если робот не способен разобраться — это его проблемы.

Почему несовершенство программного продукта должно требовать подстройки под него совсем не связанных с ним проектов?
Total votes 48: ↑17 and ↓31-14
Повторяю: Так считать не очень конструктивно.

Я тоже хочу, чтобы робот был всемогущ. Но пока недоросли технологии.
Total votes 23: ↑19 and ↓4+15
Предлагаю привести 5 примеров ложных срабатываний на хорошем коде (реального приложения). И мы обсудим здесь возможные действия.
Total votes 8: ↑7 and ↓1+6
С великим удовольствием. Хорошего кода — завались. Осталось установить и запустить анализатор.

apt-get install что?

Total votes 42: ↑15 and ↓27-12
cppcheck, например. У него тоже есть возможность убирать ложные предупреждения.
Total votes 4: ↑4 and ↓0+4
Хороший код — который хорошо работает, а не который радует глаз программиста эстетическим совершенством.
Total votes 7: ↑3 and ↓4-1
Смею не согласиться, так как код пишется программистами и для программистов a = 0; например, а машине сойдет и: xor eax,eax
Total votes 3: ↑3 and ↓0+3
a = 0 — это не для «хорошего кода», а для «удобного сопровождения».
This comment wasn’t rated yet0
Собственно, что я и сказал, ведь код сопровождают программисты и чаще всего не те кто его писал
This comment wasn’t rated yet0
Одно другому не мешает.
This comment wasn’t rated yet0
Я не поленился и написал статью о ложных срабатываниях. Готов обсудить разметку кода с помощью комментариев. Как по мне, практически все комментарии вполне по делу.
Работа с ложными срабатываниями в PVS-Studio и CppCat.

This comment wasn’t rated yet0
Поясните, что Вы хотели сказать. В PVS-Studio присутствует ряд различных механизмов для подавления ложных срабатываний. Не только комментарии.
Total votes 4: ↑2 and ↓20
там вопрос был про то, как прятать предупреждения, заведомо известные пользователю как false positive, на последующих прогонах анализатора без внесения аннотаций в исходники.
суть ссылки сводилась к перечислению некоторых возможных методов (автором из коверити), последний из которых (эвристическое сопоставление новых предупреждений с уже размеченными) даёт весьма достойные результаты, не требуя внесения в исходники.
с pvs и его механизмами подавления не знаком, но механизм аннотаций подразумевает, что подобного там нет.
Total votes 5: ↑5 and ↓0+5
Если программист ленив и не хочет сделать код более понятным анализатору (а как следствие — он всегда становится понятней и читающему кода), то да — надо использовать комментарий. Как вариант — хитрый хак. Тогда тоже, нужен комментарий. Не вижу проблемы. В 95% то место, куда указал анализатор — пахнет. Не обязательно ошибка, но попахивает. Почти всегда, самое лучшее — устранить причин запаха.
Total votes 13: ↑12 and ↓1+11
Никакого надругательства не будет, если продублировать комментарий для робота комментарием для человека.

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


Другие способы есть. Но и этот — достаточно удачное решение.
Total votes 4: ↑3 and ↓1+2
Чтобы жить с причудами дебиана и прочих, страдающих Столлманом головного мозга — можно, например, сделать маппинг одних комментариев в другие. :)

Скажем, пишется нейтральный комментарий вида // this foo bar is intentional, а в файле настроек проекта анализатора указывается, что это соответствует подавлению предупреждений 123 и 456.
Total votes 5: ↑3 and ↓2+1
Почему бы не использовать прагмы, которые игнорируются компилятором?
This comment wasn’t rated yet0
С причудами Дебиана, кстати, справиться несложно: комментарии для PVS Studio можно расставить в отдельном бранче и перед проверкой каждый раз обновлять этот бранч из master.
Total votes 2: ↑1 and ↓10
Я бы не назвал этот процесс «несложным». Потому что статический анализатор должен применяться каждым программистом, а не быть встроенным в билд-сервер. То есть надо именно в этом бранче работать, а master обновлять из него, вырезая добавление комментариев из истории.
This comment wasn’t rated yet0
Собственно, 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 не нужен ни для компиляции, ни для запуска, так что вроде бы с этим нет проблем. Что я упустил?
Total votes 6: ↑6 and ↓0+6
Ничего вы не упустили, так и есть. А вот начинать искать обходные пути еще до заглядывания в Debian Policy Manual — явно не стоило)
Total votes 1: ↑1 and ↓0+1
Безусловно. В своё оправдание скажу, что два часа назад с телефона у меня debian.org не открылся, и я решил покамест поверить amarao на слово :-)
Total votes 1: ↑1 and ↓0+1
Насчет опечатки №4. Я правильно понял? Эффект Доплера? Как он в браузере-то используется?
Total votes 5: ↑5 and ↓0+5
Как вы политкорректно называете копипасту опечатками)
Total votes 2: ↑1 and ↓10
Копипаста — причина, опечатка — следствие. Не всякая копипаста дает в результате что-то ошибочное.
Total votes 5: ↑5 and ↓0+5
Сборка осуществляется с помощью make-файлов. Поэтому просто взять и проверить проект нельзя.

Вы рассматривали вариант передать ваш анализатор в качестве компилятора в конфигуратор, чтобы он сам вызывал уже реальный компилятор?
Что-то вроде: 
CXX=<pvs-binary> ./configure
Если да, то какие трудности с этим были?
Total votes 2: ↑2 and ↓0+2
Трудность тут одна — в файле может и не быть переменной CXX.
Total votes 1: ↑1 and ↓0+1
Да в принципе примерно так и делается тоже конечно же. Вот здесь описано как в makefile встроить.
This comment wasn’t rated yet0
Прочитал подробнее про ваш способ «мониторинга» вызовов компилятора — звучит как-то ненадёжно. А если параллельно идёт несколько сборок, как ваша система узнает, какие вызовы нужно отлавливать? Я бы первым делом рассмотрел вариант с модификацией окружения. Я плохо знаю возможности Windows, но скорее всего там, как минимум, можно модифицировать переменную PATH и подставить свои программы, которые потом просто продублируют вызов уже с реальными компиляторами, линкерами и т.д.
This comment wasn’t rated yet0
Как я понял, это вариант исключительно для однократного ручного применения, для случая, когда надо сначала проверить проект — а потом уже думать, стоит ли вообще использовать статический анализатор. И для такого сценария использования этот вариант выглядит действительно круто.
Total votes 3: ↑3 and ↓0+3
Все верно, для регулярного использования надо применять другие способы. А для разового — самое оно.

Почему для разового запуска сделан специальный отдельный метод? Чтобы упростить первый запуск, знакомство с инструментом.
This comment wasn’t rated yet0
У вас уже спрашивали, но как скоро вы собираетесь проверить PVS-Studio при помощи PVS-Studio?
Total votes 23: ↑3 and ↓20-17
Какое нелепое самоубийство.
Total votes 16: ↑15 and ↓1+14
В конце статьи:

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

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

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

Пишу комментарий только ради того что бы люди задумались над простой мыслью — пишите код так что бы он проще читался и ваши волосы станут мягкими и шелковистыми :)
Total votes 3: ↑3 and ↓0+3
K switch’у должно прилагаться грамотно проставленные типы. Если в проекте int вместо enum, то у switch’а испаряется «встроенный контроль».
This comment wasn’t rated yet0
Это гораздо проще контролировать, к тому же за использование int в таких целях обычно дрючат везде, что есть правильно, ну по крайней мере я всегда за такое дрючу, по доброму конечно выражаю кюю и говорю, что хорошо бы переписать более однозначно пока это мало где используется :) да и помимо ограничения набора вариантов с помощью enum, switch будет ругаться даже на двойную проверку одного и тоже значения в int, пусть и предупреждением, но будет.
This comment wasn’t rated yet0
Sign up to leave a comment.

Your account

Sections

Information

Services

Support
© 2006–2024, Habr