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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 4 08:37:30 PDT 2016


alexfh added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2486
@@ +2485,3 @@
+
+  return (UnderlyingDecl != nullptr &&
+          InnerMatcher.matches(*UnderlyingDecl, Finder, Builder));
----------------
mboehme wrote:
> I was trying to match the style of the next matcher below -- or do you think I should establish a precedent / preference for emitting the superfluous parentheses?
Yes, I consider parentheses with `return` are confusing - I'm immediately trying to find a reason, why they are needed.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2855
@@ +2854,3 @@
+///     bar(t);
+///   }
+/// \endcode
----------------
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`


https://reviews.llvm.org/D23004





More information about the cfe-commits mailing list