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

Reply via email to