aaron.ballman added a comment.
I am not particularly comfortable with allowing type attributes to be applied
via `#pragma clang attribute`; the interface for that pragma is centered around
declarations and not types, and I'm not certain how viable the idea is to
shotgun blast out type attributes in practice.
For example, the way things are in this patch gives the user control over what
*declarations* to apply the type attribute to. That's already a little confused
-- these are type attributes, so (in general) they apply in places where types
are named even when no declaration is involved: sizeof(type), alignof(type),
etc. As a concrete example, this makes it easy to apply a calling convention
type attribute to a function declaration, but would be very hard to also apply
it to type casts. But this patch does not give the user control over what
*types* to apply the type attribute to, either. Instead, it likely relies on
the "if it can't be applied, it won't be applied" logic, which could get very
costly for other type attributes. Consider a nullability type attribute that
can only be applied to a pointer type -- there's no way to limit the
application to just pointer types, let alone to just pointer types of a
specific type.
I'd like some more justification as to why this functionality should be
supported. If there's a strong case to be made for support it, then I think we
can consider how the interface needs to change to support it. But as it stands,
I'm not sold on that this is something we should do.
================
Comment at: clang/test/AST/address_space_attribute.cpp:38-39
+#pragma clang attribute push (__attribute__((address_space(5))),
apply_to=variable(is_parameter))
+void func3(volatile int g) {}
+// CHECK: ParmVarDecl {{.*}} 'volatile __attribute__((address_space(5))) int'
+#pragma clang attribute pop
----------------
This looks like a bug -- address space attributes do not typically apply to a
parameter: https://godbolt.org/z/rrfh5bnMe
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117931/new/
https://reviews.llvm.org/D117931
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
