[PATCH] D84297: [clangd] Fix Origin and MainFileOnly-ness for macros

Wed Jul 22 02:24:25 PDT 2020

kadircet added a comment.

> the test.h in the patch description is missing a #define X.

ah right, git descriptions omitting lines starting with `#` fixed it for include, but missed the define :face_palm:

================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:383
+      !isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
+                    ASTCtx->getLangOpts());

----------------
hokein wrote:
> nit: move this var to Line 398, in some cases (builtin macros), it is not used, so would save some cost.
> 
> this is duplicated with the one in `handleDeclOccurrence`, creating a new function seems not worthy...
yeah and these are slightly different, the one in declOccurence makes sure the decl is written inside the main file, whereas this one also accepts remappings through `#line` directives.

not sure if it is necessary, but it was the old behavior, so I am just keeping it, no tests seem to break and we already do not index builtin macros or the ones spelled in a builtin-file. I don't think line directives are widely used within the rest, but again such a change would deserve a separate patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84297