ymandel added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:108
+      Diag << Inserter.createIncludeInsertion(
+          Result.SourceManager->getFileID(T.Range.getBegin()), T.Replacement);
       break;
----------------
alexfh wrote:
> Can this be a macro file id? I'd suggest to add tests (probably for checks 
> using this functionality) with a few nested includes and fixes in normal 
> code, code in macros declared and expanded in different files, locations in 
> macro bodies, macro arguments, and some tricky cases like fix pointing to a 
> pasted token.
> Can this be a macro file id? I'd suggest to add tests (probably for checks 
> using this functionality) with a few nested includes and fixes in normal 
> code, code in macros declared and expanded in different files, locations in 
> macro bodies, macro arguments, and some tricky cases like fix pointing to a 
> pasted token.

Unlikely, given that API that populates the location. Most use a default verion 
which explicitly maps the location to the expansion location. However, it is 
possible. Also, Transformer rejects changes inside macros (except arguments) so 
that would also be hard to engineer.

Still, I'll look into better tests in the check itself. I did try that but 
mostly failed because the lit tests don't support checking anything in headers. 
However, the macro checks sound worth it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96542

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

Reply via email to