[PATCH] D112608: [clangd] IncludeCleaner: Do not process locations in built-in files

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!



================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:137
+    // Check if Loc is not written in a physical file.
+    if (FID.isInvalid() || SM.isWrittenInBuiltinFile(Loc))
       return;
----------------
I couldn't figure out why macros defined via CLI are ending up in `<built-in>` file, despite having the right line directive in https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1257. Looks like a bug in `SourceManager`. But still let's check for that too, just to be on the safe side (i.e. `isWrittenInCommandLineFile`)


================
Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:245
 
+    int y = CLI;
+
----------------
you can change this to `int y = CLI + __llvm__` to explicitly check for both CLI and built-in macros.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112608