kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
Thanks! mostly LGTM, only confused about changing symbolcollector to produce a unique header per symbol(where as it was(could) produce multiple headers before). What is the rationale behind that one? ================ 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. ---------------- This is losing ability to store multiple header files. Is that intentional? ================ Comment at: clangd/unittests/SymbolCollectorTests.cpp:1043 + int decl(); + #define MACRO + ---------------- can you also add a case where we first see `MACRO` and then `decl`. 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