[PATCH] D108560: [clang-tidy] Add support for `NOLINTBEGIN` ... `NOLINTEND` comments

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.


================
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)) {
----------------
aaron.ballman wrote:
> `List` is the same on every iteration through the loop, so we might as well set it once and reuse it.
I've refactored this so that the logic to select the appropriate list (the ternary operation) is only defined once. 
However, this logic still has to be called within the `for` loop, not outside, because `SuppressionIsSpecific` can change on every iteration of the loop.

e.g.
```
// NOLINTBEGIN(check)  <-- for loop iteration 0: SuppressionIsSpecific == true,  &List = SpecificNolintBegins
// NOLINTBEGIN         <-- for loop iteration 1: SuppressionIsSpecific == false, &List = GlobalNolintBegins
// NOLINTEND           <-- for loop iteration 2: SuppressionIsSpecific == false, &List = GlobalNolintBegins
// NOLINTEND(check)    <-- for loop iteration 3: SuppressionIsSpecific == true,  &List = SpecificNolintBegins
```


================
Comment at: clang-tools-extra/docs/clang-tidy/index.rst:367
+
+All ``NOLINTBEGIN`` comments must be paired by an equal number of ``NOLINTEND``
+comments. Moreover, a pair of comments must have matching arguments -- for
----------------
Additional documentation added here about the new diagnostic and how it is triggered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108560