[PATCH] D108560: [clang-tidy] Add support for NOLINTBEGIN ... NOLINTEND comments to suppress clang-tidy warnings over multiple lines

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D108560#3020444 <https://reviews.llvm.org/D108560#3020444>, @salman-javed-nz wrote:

> Also, in your example, you have begun all checks (`check`, `check2`, `check3` ... `checkN`) but only ended one of them. The remaining checks are still awaiting a `NOLINTEND` comment of some sort.

+1

> I say all this in the interest of keeping the Clang-Tidy code simple, and reducing the amount of weird behavior that is possible (due to mistakes, lack of knowledge, or "creative" use of the feature). I rather this feature be strict and limited in scope, rather than flexible enough to be used in unforeseen error-prone ways.

Thank you for the careful evaluation -- I agree with you!

> Perhaps this feature can be extended in the future (if need be) after it gets some use and user feedback comes in?

Agreed.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:400-401
+  bool SuppressionIsSpecific;
+  for (const auto &Line : Lines) {
+    if (isNOLINTFound("NOLINTBEGIN", CheckName, Line, &NolintIndex,
+                      &SuppressionIsSpecific)) {
----------------
`List` is the same on every iteration through the loop, so we might as well set it once and reuse it.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:404-406
+      auto &List =
+          SuppressionIsSpecific ? SpecificNolintBegins : GlobalNolintBegins;
+      List.emplace_back(LinesLoc.getLocWithOffset(NolintIndex));
----------------



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:410-412
+      auto &List =
+          SuppressionIsSpecific ? SpecificNolintBegins : GlobalNolintBegins;
+      if (!List.empty()) {
----------------



================
Comment at: clang-tools-extra/docs/clang-tidy/index.rst:306-308
+line*. The ``NOLINTBEGIN`` and ``NOLINTEND`` comments allow suppressing
+clang-tidy warnings on *multiple lines* (affecting all lines between the two
+comments).
----------------
We should also document the diagnostic behavior of the comments themselves (the fact that misusing these can generate diagnostics).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108560/new/

https://reviews.llvm.org/D108560