salman-javed-nz added a comment.

In D108560#3012057 <https://reviews.llvm.org/D108560#3012057>, @aaron.ballman 
wrote:

> Thanks, I think this is getting close! There are two more test cases that I 
> think are interesting (and should cause any issues, hopefully):
>
>   // NOLINTEND
>   // CHECK-MESSAGES: for diagnostic on the previous line
>
> and
>
>   // CHECK-MESSAGES: for diagnostic on the subsequent line
>   // NOLINTBEGIN
>
> as the only contents in the file. The idea is that we want to check that 
> searching for a "begin" when seeing an "end" at the top of the file doesn't 
> do anything silly like try to read off the start of the file, and similar for 
> "end" when seeing a "begin" at the end of a file.

The good news is that the program does not do anything silly like read off the 
boundaries of the file. :-)
What is noteworthy, however, is that in 
`test/clang-tidy/infrastructure/nolintbeginend-begin-at-eof.cpp` only the 
original clang-tidy check diag is shown, not the diag about the unmatched 
NOLINTBEGIN. This is because of the early exit in `lineIsWithinNolintBegin()`:

  // Check if there's an open NOLINT(BEGIN...END) block on the previous lines.
  // ...
  auto Error = tallyNolintBegins(Context, SM, CheckName, PrevLines,
                                 FileStartLoc, NolintBegins);
  // ...
  if (NolintBegins.empty())
    return false;
  
  // Check that every block is terminated correctly on the following lines.
  // ...

This has been fixed in the patch I just posted.

As for your latest message about NOLINTBEGIN(check) ... NOLINTEND(other-check), 
I'll have a think about it and get back to you in a day or two.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108560

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to