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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 4 09:11:17 PDT 2016

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!


More information about the cfe-commits mailing list