On Wed, Aug 29, 2018 at 8:41 PM, Richard Smith - zygoloid via Phabricator <revi...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
