[PATCH] D54404: Exclude matchers which can have multiple results

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 19 05:21:42 PST 2018


aaron.ballman added inline comments.


================
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:624
+      "hasAnyDeclaration",
+      "hasAnyName",
+      "hasAnyParameter",
----------------
steveire wrote:
> aaron.ballman wrote:
> > steveire wrote:
> > > sbenza wrote:
> > > > I'm not sure what goes in this list.
> > > > `hasAnyName` is here but not `hasName`.
> > > > What is ambiguous about `hasAnyName`?
> > > I have a follow-up which adds output showing that `hasName` can be used. See
> > > 
> > > http://ec2-52-14-16-249.us-east-2.compute.amazonaws.com:10240/z/_X9mnw
> > > 
> > > If there was an entry there for `hasAnyName`, what would go in it?
> > > If there was an entry there for hasAnyName, what would go in it?
> > 
> > Presumably the same as `hasName()`, though for the purposes of that list, I could see why it would be a bit odd to list it.
> > 
> > It almost feels like this isn't about ambiguity of the matchers (at least, not always) so much as it is about sensibility within a "the following are related matchers" list due to there being many different ways for matchers to relate. For instance, a related matcher could be `functionDecl(hasAnyParameter(anything()))`, but you might not want to list that because it's an open-ended problem to generate all such cases and it has very limited value to list them. Is that a better way for me to think about this?
> >  it is about sensibility ... Is that a better way for me to think about this?
> 
> Yes. The list attempts to exclude things that don't make sense to show for `matcher` output.
Okay, so this isn't really about ambiguity as I was thinking of it (I was thinking of it in terms of "calling this matcher with this argument would be ambiguous"). Perhaps a comment like:
```
When matcher output is enabled, a list of related matchers is displayed to the user suggesting other matchers that would also be true for the matched construct. This is a list of matchers to exclude when considering those related matchers. Add elements to this list when the relationship to other matchers is impossible to determine or expected to be uninteresting to the user.

For instance, when matching functionDecl() to a function declaration with a single parameter, it would not be interesting for the user to know that functionDecl(hasParameter(anything())) also matches. Given that the anything() matcher is unlikely to be of interest in any matcher relationship, it is listed below as one of the matchers to exclude.
```
(Feel free to reword, come up with more compelling examples, etc.)


Repository:
  rC Clang

https://reviews.llvm.org/D54404





More information about the cfe-commits mailing list