[PATCH] D61442: [clangd] Fix header-guard check for include insertion, and don't index header guards.

Fri May 3 04:15:51 PDT 2019

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