[PATCH] D30009: Add support for '#pragma clang attribute'

Aaron Ballman via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 30 10:55:49 PDT 2018


On Wed, Aug 29, 2018 at 8:41 PM, Richard Smith - zygoloid via
Phabricator <reviews at reviews.llvm.org> wrote:
> rsmith added a comment.
> Herald added a subscriber: llvm-commits.
>
> One consequence of this patch (see https://reviews.llvm.org/rL341002) is that adding documentation to an existing attribute //changes the behavior of Clang//. That does not seem acceptable to me: documentation improvements should not change which attributes we allow this pragma to appertain to.
>
> If we really want an "only documented attribtues can use this feature" rule (and I'm really not convinced that is a useful rule to enforce), I think the best way to do that is to add another flag on the Attr instance in the .td file to opt the attribute out of `#pragma clang` support, opt out all the undocumented attributes, and add an error to TableGen if we find that an undocumented attribute has `#pragma clang` support enabled.

I think this is a reasonable approach, but I'm also not as convinced
we need the attributes to be documented in order to use this pragma
any more. The thought process behind why it was implemented this way
(from memory, so forgive me if I'm fuzzy) was because a blanket
application of an attribute through a pragma makes for hard to
maintain code when the attributes are undocumented because the user
has no way of knowing what will actually receive that attribute. They
also don't know which attributes are unsupported in this form (like
late parsed attributes, or ones with only a __declspec spelling, etc),
but that seems like less of a concern as we err if the attribute is
not supported.

~Aaron


More information about the llvm-commits mailing list