[PATCH] D43823: [clangd] Prefer the definition of a TagDecl (e.g. class) as CanonicalDeclaration.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/index/SymbolCollector.cpp:260
       BasicSymbol = addDeclaration(*ND, std::move(ID));
+    else if (isPreferredDeclaration(OriginalDecl, Roles))
+      BasicSymbol = addDeclaration(OriginalDecl, std::move(ID));
----------------
sammccall wrote:
> This is a bit subtle and I think comment-worthy.
> We're doing a bunch of duplicated work, but at most once because we expect to see only one preferred declaration per TU, because in practice they're definitions?
> 
> An alternative would be to have addDeclaration loop through the redecls to try to find a preferred one (and use the passed in one if none is found). This would avoid any duplicated work. Up to you.
The reason why I am a bit hesitant to use `redecls` is that we wouldn't know whether all re-declarations would have been indexed (e.g. there might be implicit decls that were dropped by the index library) and passed the filters we had (e.g. `shouldFilterDecl`). And you are right, the duplicated work should usually be little. I added a comment to clarify the duplicated work.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43823