sammccall added a comment. (while this is useful to you, let's keep discussing, but I'm also happy to stop and land this with your preferred API/semantics - just LMK if changes are needed)
In D89743#2360032 <https://reviews.llvm.org/D89743#2360032>, @aaron.ballman wrote: > Thank you for giving this feedback, it's helpful! I may have to rethink this > a bit because it never dawned on me that someone would think of attributes > being more like a `DeclRefExpr` than a `NamedDecl`. If I'm following along > properly, this is because the AST node for the attribute could have been > generated from various ways of spelling it? e.g., `gcc::aligned` vs > `_Alignas` vs `align` vs `alignas` all making an `AlignedAttr`. My mental model is that the Attr **classes** own the names, and the Attr **instances** refer to the classes using the names. So the class `AlignedAttr` has the name `aligned`, and the instance `[[gnu::aligned(8)]]` refers to it. Just like the `FunctionDecl` `double sqrt(double);` has the name `sqrt`, and the DeclRefExpr in `sqrt(8)` refers to it. To reinforce this, I can't choose a different name for the instance: `[[gnu::whatever]]` doesn't produce an Attr instance at all. But I can choose a different name for the class (by modifying Attr.td). So an Attr instance (what we're matching) feels more like a reference than a primary entity. (Similarly, the **semantics** are associated with the Attr class rather than the instance). If clang's AST modeled syntax rather than semantics, I imagine there'd be one `Attr` class and each instance could have an arbitrary name and arg list. In that case an Attr would seem to "have" a name in the same sense a decl does. > I figured it would make more sense for `attr(hasName("aligned"))` to match > any `AlignedAttr` regardless of how the user wrote it in their source code, > but perhaps others have different ideas here. That definitely seems like the common case. However I can certainly imagine wanting to e.g. write a migration from `__attribute__((xxx))` to `[[gnu::xxx]]`. (If attr **classes** were AST nodes too, like DeclRefExpr/Decl or TypeLoc/Type then you'd resolve this by using a matcher on one or the other) As it stands it seems too surprising that `Attr::getAttrName()` returns the particular name used for that instance, but `attr(hasAttrName("..."))` would match any name associated with the same class. Maybe `attr(equivalentAttrName("..."))` or so, possibly constrasting to `attr(exactAttrName("..."))`? > The scope handling is a bit of a question mark. Attributes don't really have > namespaces like other identifiers, but they sort of have them when you squint > at the name just right. For instance, the scope handling currently allows you > to match `::foo` in C mode even though C doesn't have the notion of > namespaces. So I thought it wouldn't be unintuitive for it to also work for > namespace scopes as well. Yup, I don't have a real opinion here, I just assumed it was different. Seems like it would work fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89743/new/ https://reviews.llvm.org/D89743 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
