[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector<const NamedDecl *, 1>
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+                         DeclRelationSet Mask = {});
----------------
ilya-biryukov wrote:
> No need to fix this.
> 
> The name could probably be better, but we can fix this later. Don't have any good ideas.
I think as a public API this should be clearer on how/when to use and its relation to other things (planning to send a patch, but wanted to discuss a bit here first).

- This is essentially a filter of allTargetDecls (as is targetDecl), but the relationship between the two isn't clear. They should have closely related names (variant) or maybe better be more orthogonal and composed at the callsite.
- The most distinctive word in the name is `explicit`, but this function doesn't actually have anything to do with explicit vs non-explicit references (just history?)
- It's not clear to me whether we actually want to encapsulate/hide the preference for instantiations over templates here, or whether it should be expressed at the callsite because the policy is decided feature-by-feature. `chooseBest(...)` vs `prefer(TemplateInstantiation, TemplatePattern, ...)`. The latter seems safer to me, based on experience with targetDecl so far.

A couple of ideas:
 - caller invokes allTargetDecls() and then this is a helper function `prefer(DeclRelation, DeclRelation, Results)` that mutates Results
 - targetDecl() becomes the swiss-army filter and accepts a list of "prefer-X-over-Y", which it applies before returning the results with flags dropped


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596