Attribute spelling policy

[Aaron Ballman via cfe-commits]

On Mon, Oct 23, 2017 at 11:16 AM, Arthur O'Dwyer
<arthur.j.odwyer at gmail.com> wrote:
> On Mon, Oct 23, 2017 at 7:33 AM, Aaron Ballman via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>>
>> On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>> > On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote:
>> >>
>> >> Attributes come with multiple spelling flavors, but when it comes to
>> >> adding new attributes that are not present in other compiler tools
>> >> such as GCC or MSVC, we have done a poor job of being consistent with
>> >> which spelling flavors we adopt the attributes under. Some of our
>> >> attributes are specified with only an __attribute__ spelling (about
>> >> 100), while others are specified with both __attribute__ and
>> >> [[clang::XXX]] (about 30), and still others are specified as only
>> >> [[clang::XXX]] attributes (only 1). This puts additional burden on
>> >> developers to remember which attributes are spelled with what syntax
>> >> and the various rules surrounding how to write attributes with that
>> >> spelling.
>> >>
>> >> I am proposing that we take a more principled approach when adding new
>> >> attributes so that we provide a better user experience. Specifically,
>> >> when adding an attribute that other vendors do not support, the
>> >> attribute should be given an __attribute__ and [[clang::]] spelling
>> >> unless there's good reason not to. This is not a novel proposal -- GCC
>> >> supports all of their documented __attribute__ spellings under a
>> >> [[gnu::XXX]] spelling, and I am proposing we do the same with our
>> >> vendor namespace.
>> >
>> > For attributes that both Clang and GCC support, where GCC provides a
>> > [[gnu::X]] syntax, do you propose that our policy will be to support the
>> > same?
>>
>> Yes; we should use the [[gnu::X]] spelling instead of adding the same
>> attribute under [[clang::X]] unless we think our semantics need to
>> significantly differ from GCC's.
>
>
> This seems like a departure from what you wrote originally, unless you just
> misspoke or I misinterpreted.
> You originally said, "[New attributes] should be given an __attribute__ and
> [[clang::]] spelling unless there's good reason not to."  I would 100% agree
> with that policy, from a user's point of view. It would be awesome to know
> exactly how an attribute is spelled without having to look it up in the
> manual.

Agreed.

> But when you used the word "instead" just now, you implied that the policy
> would be more like "New attributes should be given an __attribute__ and
> [[clang::]] spelling unless there's good reason not to, or unless Clang is
> copying work originally done by the GNU project, in which case the new
> attribute should be given __attribute__ and [[gnu::]] spellings but not a
> [[clang::]] spelling." Is that really what you meant, or was your original
> statement closer to what you meant?

That is really what I meant, but to be extra-double-clear:

* If we're adding a new attribute that has never existed under a
previous [[vendor::attr]] spelling, it should be __attribute__ and
[[clang::attr]].
* If we're adding a new-to-clang attribute that has existed under a
previous [[vendor::attr]] spelling:
  * if we intend for the attribute semantics to match the other
vendor, it should be [[vendor::attr]].
  * if we intend for the attribute semantics to differ from the other
vendor, it should be [[clang::attr]].
  * whether it's also __attribute__ may depend on the attribute being
added; for instance, we may or may not want to add an __attribute__
spelling for a [[gsl::attr]].

> As a user, I would strongly prefer to write all my attributes in a
> consistent way. If that way has to be __attribute__((foo)), then okay. But
> in C++11-and-later, it would be nice to write all my attributes as
> [[clang::foo]], and not have to keep consulting the manual to find out which
> of [[foo]], [[clang::foo]], and [[gnu::foo]] is the proper spelling for this
> particular attribute.

You will always have to figure out whether it's [[foo]] or
[[vendor::foo]] because [[foo]] is provided by the standard. However,
the point still stands of it being difficult to remember [[gnu::foo]]
vs [[clang::foo]]. However, I think that's a momentary bit of
confusion compared to the confusion of "where do I write this
attribute so that it works" that you get with __attribute__.

> Of course the worst possible outcome would be where [[clang::foo]] and
> [[gnu::foo]] both exist and have different semantics. I hope that doesn't
> happen!  But if it has already happened, then it becomes even more important
> that a user of Clang never need to use the [[gnu::]] spelling for any Clang
> attribute (because accidentally using [[gnu::]] on the wrong attribute could
> cause bugs, in the hypothetical case that [[clang::foo]] and [[gnu::foo]]
> have different semantics).
>
> Basically, as a user, I would like to see:
> - Attribute names never deliberately collide with other vendors

While that would be ideal, it also seems unlikely to be enforceable.
One point to vendor namespaces is specifically so that collisions can
be managed. I agree that we should not *deliberately* collide with
drastically different semantics, however.

> - Every attribute supported by Clang is available under __attribute__
> - Every attribute supported by Clang is available under [[clang::]]
> - Any attribute that mimics a GNU feature is available under [[gnu::]] (and
> also under the two above)
> - Any attribute that mimics an MSVC feature is available under [[msvc::]]
> (and also under the first two above)

Unless I've missed something recently, I don't know that Microsoft has
added any vendor-specific attributes so I'm not certain about the
[[msvc::]] name, but I agree that any attributes we implement from
their namespace should match their namespace name. However, I disagree
that the name should be automatically provided under __attribute__.
For instance, we do not provide __declspec attributes under
__attribute__ unless GCC has already done it for some reason.

> - Any attribute that mimics a GNU-and-MSVC feature (thus, a feature
> available on all three platforms) is available under all of [[gnu::]],
> [[msvc::]], and [[clang::]] (and also under __attribute__)
>
> Then, as a user, I don't need to care about the historical accident of which
> project's employees invented a feature first; I can just use the [[xxx::]]
> prefix corresponding to my primary compiler, and either it will work or it
> will error out (in which case I'll know that my primary compiler does not
> support that feature).

My original intent was to not add [[clang::attr]] simply because we
also support [[gnu::attr]]. My biggest concern with doing that is
users then rely on clang::attr and gnu::attr being exact aliases with
identical semantics and I'm not certain that's correct. Differing
semantics would not be great, but it does seem likely to happen (we've
diverged from GCC behavior on some of their features in the past when
it is preferable for us to do so). By keeping the original vendor
namespace intact, it becomes obvious to the user as to what semantics
to expect. i.e., if the user compiles their code with [[gnu::foo]]
under GCC and Clang and the behavior *differs*, then this is a Clang
bug because Clang should be honoring the GCC implementation with their
vendor namespace attributes. If we automatically add all [[gnu::]]
attributes to [[clang::]], we're back to playing "whose bug is it?"
for attributes whose semantics differ slightly on purpose.

Your scenario works nicely if the user only ever cares about writing
code with Clang, but I'm also concerned about users who compile their
code with both Clang and GCC. GCC doesn't support all our common
attributes under [[clang::]], so I still have to know which variant to
use in my code.

Thank you for bringing up this point, because I think it's worth discussing.

~Aaron