kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/Headers.cpp:126 + bool HandleComment(Preprocessor &PP, SourceRange Range) override { + if (!isInsideMainFile(Range.getBegin(), SM) || Range.getBegin().isMacroID()) + return false; ---------------- we're already keeping track of being inside builtinfile today, through `FileChanged` callbacks, can you also keep track of being inside main file rather than doing the check for each comment we hit? ================ Comment at: clang-tools-extra/clangd/Headers.cpp:149 + // The last line "IWYU pragma: keep" was seen in the main file. + int PragmaKeepInMainFile = -1; }; ---------------- maybe `LastPragmaKeepInMainFileLine` also mention in the comments that this is 0-indexed? ================ Comment at: clang-tools-extra/clangd/Headers.h:130 + // IWYU pragmas but it needs to be explicitly added to as a preprocessor + // comment handler. PP wants to own the PPCallbacks, so the typical way to + // do both is: ---------------- kbobyrev wrote: > sammccall wrote: > > Alternatively we could just give up on having a pure-ish API and say `void > > collect(Preprocessor&)`. > > Again there's not really any other sensible way to use it. > `PrecompiledPreamble.cpp` requires PPCallbacks being returned directly, so I > think we can't easily have `void IncludeStructure::collect(PP &). as discussed offline let's do that inside `BeforeExecute` callback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114072/new/ https://reviews.llvm.org/D114072 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits