[PATCH] D117931: [Clang] Support `address_space` attribute in `#pragma clang attribute

[Aaron Ballman via Phabricator via cfe-commits]

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