[PATCH] D126138: [clang-tidy] Fix #55134 (regression introduced by 5da7c04)

Salman Javed via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun May 22 03:22:40 PDT 2022


salman-javed-nz added a comment.

In D126138#3529820 <https://reviews.llvm.org/D126138#3529820>, @paulaltin wrote:

> Thanks for preparing this revision @salman-javed-nz!
>
> Do you think it could be worth adding a few more test cases to cover this? It turned out that this issue wasn't actually specific to multi-line macros (see this comment <https://github.com/llvm/llvm-project/issues/55134#issuecomment-1112522063>), so if the test case on line 96 was passing then it must not be fully/correctly testing NOLINT for single-line macros. I guess the only way to do this would be to add more checks to the nolint.cpp file, but I realise that's not a trivial change.

I added a second test case to this patch, to cover the specific check mentioned in the GitHub ticket.
This fix is check-agnostic, so I don't think we need to add even more tests than the two proposed here.

I think I've addressed what everyone has discussed in this review up to this point. Let me know what other updates I can make to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126138



More information about the cfe-commits mailing list