[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK, I'm going to drop hasAttrName from the patch (it's a relatively tiny piece of the code/tests) and land this, and put that back up as another review for discussion. And thanks, it's been interesting and I learned a bunch!
We didn't talk about overloading isImplicit to apply to attrs too, but it doesn't seem like that was controversial (and it does help with the tests).

In D89743#2408747 <https://reviews.llvm.org/D89743#2408747>, @aaron.ballman wrote:

>>> Maybe `attr(equivalentAttrName("..."))` or so, possibly constrasting to `attr(exactAttrName("..."))`?
>
> I think this design is the most flexible and will be the most understandable in the long-run because it makes it clear which name-matching behavior is being used. But do we want to stick `attr` in the name of this to limit it to just attributes or do we think this is a generalized concept that should apply to matching other names as well? e.g., `hasType(equivalentName("long"))` to match either `long`, `long int` or `signed long int` and `hasType(exactName(`long int`))` to only match `long int` and not the other forms? Or do you think this is a bridge too far because it would be reasonable to expect `hasType(equivalentName("long"))` to match typedef names because those are equivalent (or perhaps more problematically, would the user expect `hasType(equivalentName("struct foo<bar>::baz<quux>"))` to match as an equivalent name for the parameter type (spelled with a `using` declaration) in `void foo(blah b)`)? (I think I've just about talked myself into keeping `attr` in the name so we don't try to use this for other kinds of identifiers with equivalent names.)

Putting attr in the name is the *safe* option - as we've seen, going for a generic name and hoping it can work consistently for all types over time is risky.
(The cost of a confusing "overload" set is also high because the polymorphism mechanism is poorly understood - most users probably can't easily look up the variant they're using and ignore the others).
On the other hand, when it is consistent, I think the uniformity is nice - so a generic name isn't without merit, it's just a bit of a gamble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743