[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked an inline comment as done.
kadircet added a comment.

thanks, this looks good in general, but it would be nice to make sure we are not blowing the index memory and disk usage.

could you grab some numbers for these two before/after this patch?



================
Comment at: clang-tools-extra/clangd/index/Serialization.cpp:422
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 13;
+constexpr static uint32_t Version = 14;
 
----------------
this isn't really necessary as we are not changing the serialization format. you would need to delete the existing clangd caches from your machine instead.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:317
   // FIXME: we should try to use the file locations for other fields.
-  if (CollectRef && !IsMainFileOnly && !isa<NamespaceDecl>(ND) &&
+  if (CollectRef && (!IsMainFileOnly || ND->isExternallyVisible()) &&
+      !isa<NamespaceDecl>(ND) &&
----------------
this will result in calculation and caching of linkage. it should be OK to do so though, as we run indexing after parsing finishes. (no action needed, just some notes for future travellers).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84513/new/

https://reviews.llvm.org/D84513