Page MenuHomePhabricator

[Clang] Support `address_space` attribute in `#pragma clang attribute
AbandonedPublic

Authored by egorzhdan on Jan 21 2022, 3:14 PM.

Details

Summary

This change adds support for type attributes (for example, address_space) in pragmas, so that the following code now compiles correctly:

#pragma clang attribute push (__attribute__ ((address_space(1))), apply_to=variable(is_global))

int var;

#pragma clang attribute pop

Since the attribute matching logic (attr::SubjectMatchRule) applies to an already constructed Decl, we first determine the declaration's type ignoring pragma attributes, then construct the Decl, and then recalculate its type if any type attribute was applied.

rdar://78269223

Diff Detail

Event Timeline

egorzhdan created this revision.Jan 21 2022, 3:14 PM
egorzhdan requested review of this revision.Jan 21 2022, 3:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2022, 3:14 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

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.

clang/test/AST/address_space_attribute.cpp
38–39

This looks like a bug -- address space attributes do not typically apply to a parameter: https://godbolt.org/z/rrfh5bnMe

egorzhdan abandoned this revision.Feb 1 2022, 11:08 AM

Thanks @aaron.ballman for your feedback. I will probably abandon this change until we have a more compelling reason to apply attributes to types via pragmas.