sammccall added inline comments.
================ Comment at: clangd/index/SymbolCollector.h:130 + // The final spelling is calculated in finish(). + llvm::DenseMap<SymbolID, FileID> IncludeFiles; + // Indexed macros, to be erased if they turned out to be include guards. ---------------- kadircet wrote: > This is losing ability to store multiple header files. Is that intentional? Careful reading of the code shows that ability never existed :-) `addDeclaration` always creates a new Symbol, sometimes populates its `IncludeHeaders`, and then replaces the existing symbol. We always find a single decl of the symbol we prefer in the TU (though sometimes it takes us a few attempts). Of course, I never read the code that carefully - I wrote this as a `vector<pair<SymbolID, FileID>>` to "preserve" the ability to add multiple headers... and then the tests failed :-) ================ Comment at: clangd/unittests/SymbolCollectorTests.cpp:1043 + int decl(); + #define MACRO + ---------------- kadircet wrote: > can you also add a case where we first see `MACRO` and then `decl`. IIUC the idea is that macros are "eager-er" than decls so more likely to break the caching. That makes sense. WDYT about just putting the macro first, so we *only* test the "hard" case. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61442/new/ https://reviews.llvm.org/D61442 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
