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