[PATCH] D53808: [clangd] Don't collect refs from non-canonical headers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

It's not obvious whether/why this is the right thing to do.

One set of reasoning that occurs to me:

- headers without guards produce code in some context-sensitive way, they only exist in the context of their including file
- for the symbols they produce, we have ways to reconcile across contexts - by symbol ID
- but for refs, there's no such context of identity
- therefore it's not clear whether a ref in an unguarded header should be tracked once, once per include, or not at all. Not at all is defensible.

This falls down a bit though when we see that in practice we'll often have refs to the same symbol, from the same location, with the same role - this is a reasonable way to reconcile.

If this is just a way to trim down index size, we should consider codebases other than LLVM, as it seems pretty likely the use of generated files there is atypical.



================
Comment at: clangd/index/SymbolCollector.cpp:479
   const auto &SM = ASTCtx->getSourceManager();
+  llvm::DenseSet<FileID> CanonicalHeaders;
+  for (auto &IT : PP->macros()) {
----------------
High-level comment - what are you doing here? Why?


================
Comment at: clangd/index/SymbolCollector.cpp:479
   const auto &SM = ASTCtx->getSourceManager();
+  llvm::DenseSet<FileID> CanonicalHeaders;
+  for (auto &IT : PP->macros()) {
----------------
sammccall wrote:
> High-level comment - what are you doing here? Why?
Use of "canonical" here and elsewhere isn't obvious to me - does this name come from somewhere?

"Modular" is how I would describe these headers - it's imprecise, but you could define it somewhere.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53808