aaron.ballman added inline comments.
================
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:299
+ size_t BracketIndex = NolintIndex + NolintMacro.size();
+ // Check if the specific checks are specified in brackets
+ if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
----------------
Comments should be complete sentences, including punctuation (here and
elsewhere).
================
Comment at: docs/ReleaseNotes.rst:259-260
+- Added an ability to specify in parentheses the list of checks to suppress
for the ``NOLINT``
+ and ``NOLINTNEXTLINE`` comments.
+
----------------
How about: "Added the ability to suppress specific checks (or all checks) in a
NOLINT or NOLINTNEXTLINE comment."?
================
Comment at: docs/clang-tidy/index.rst:253
+Generally, there is no need to suppress :program:`clang-tidy` diagnostics. If
+there are false positives, either a bug should be reported or the code should
be
----------------
I don't agree with that initial statement's use of "generally" -- checks that
are chatty live in clang-tidy, as are checks for various coding standards
(which commonly have a deviation mechanism). Also, I don't think we should
encourage users to unconditionally report false positives as bugs; many of the
coding standard related checks provide true positives that are annoying and
users may want to deviate in certain circumstances (like CERT's rule banning
use of `rand()` or `system()`). I would reword this to:
```
While clang-tidy diagnostics are intended to call out code that does not adhere
to a coding standard, or is otherwise problematic in some way, there are times
when it is more appropriate to silence the diagnostic instead of changing the
semantics of the code. In such circumstances, the NOLINT or NOLINTNEXTLINE
comments can be used to silence the diagnostic. For example:
```
I would also describe the comment syntax more formally as (my markdown may be
incorrect, you should ensure this renders sensibly), with surrounding prose:
```
*lint-comment:*
*lint-command* *lint-args~opt~*
*lint-args:*
`(` *check-name-list* `)`
*check-name-list:*
*check-name*
*check-name-list* `,` *check-name*
*lint-command:*
`NOLINT`
`NOLINTNEXTLINE`
```
Specific to the prose mentioned above, you should document where the feature is
tolerant to whitespace (can there be a space between NOLINT and the parens,
what about inside the parens, how about after or before commas, etc).
================
Comment at: docs/clang-tidy/index.rst:270
+ // Skip only the specified diagnostics for the next line
+ // NOLINTNEXTLINE (google-explicit-constructor, google-runtime-int)
+ Foo(bool param);
----------------
Is the space before the `(` intended?
https://reviews.llvm.org/D40671
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
