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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
