[PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2855
@@ +2854,3 @@
+///     bar(t);
+///   }
+/// \endcode
----------------
alexfh wrote:
> mboehme wrote:
> > > The documentation doesn't describe what the matcher does (can you please clarify the docs?).
> > 
> > I've rephrased the description of the matcher -- is it clearer now?
> > 
> > > The implementation suggests that this is looking to see if the given decl exists in the overload expression set, which makes me wonder why this isn't implemented on the hasDeclaration() traversal matcher rather than adding a new matcher name?
> > 
> > As klimek notes, the difference is that hasDeclaration() is currently used only for nodes that have exactly one associated declaration.
> > 
> > My proposal would be to stay with canReferToDecl -- thoughts?
> +1 to `canReferToDecl`
`canReferToDecl` doesn't make it any more clear that there could be multiple candidates, and I find its usage to read really strangely. However, it is a good point about `hasDeclaration` usage being singular currently. My preference is for `hasAnyDeclaration`, and the precedence is `hasAnyName`, `hasAnyConstructorInitializer`, `hasAnyArgument`, etc.

The documentation does read better now, thank you!


https://reviews.llvm.org/D23004