r199053 - Clarify warn_cxx98_compat_attribute diagnostic

[Aaron Ballman]

On Tue, Jan 14, 2014 at 3:25 AM, Alp Toker <alp at nuanti.com> wrote:
>
> On 14/01/2014 03:10, Richard Smith wrote:
>
> On Mon, Jan 13, 2014 at 6:37 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Mon, Jan 13, 2014 at 9:24 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On Sun, Jan 12, 2014 at 11:14 AM, Aaron Ballman <aaron at aaronballman.com>
>> > wrote:
>> >>
>> >> After doing a bit more research and discussion off-list, I think
>> >> "generalized attribute" is acceptable.  So patch LGTM as-is.
>> >
>> >
>> > Really? I wouldn't expect someone seeing this diagnostic to understand
>> > that
>> > "generalized attribute" means C++11 attributes (it's a really weird
>> > term,
>> > since they're not a generalization of anything). This isn't an official
>> > name
>> > for them, and doesn't distinguish them from the other attribute syntaxes
>> > we
>> > support. Given that this is a diagnostic about compatibility with C++98,
>> > "C++11 attributes" seems like the clearest way of expressing this.
>>
>> As Alp had pointed out, we document the name as "generalized
>> attribute" in our feature support documentation,
>
>
> You're right, we did. I just fixed that.
>
>>
>> and it's the original name of the feature.
>
>
> [citation needed]
>
> The proposal calls them "General Attributes for C++" (and all previous
> revisions of it did the same); the word "Generalized" seems to have been
> accidentally transferred from "Generalized constant expressions" in the GCC
> list, and we inherited that mistake when we sync'd our list with theirs in
> r142015. The paper and C++ standard both call them simply "attributes".
>
>> Also, a quick google search of "generalized
>> attribute" yielded more results than "C++11 attribute" did (not saying
>> this was particularly scientific). So that's why I gave the LGTM on
>> the term.
>
>
> Hah, it seems that lots of people copied our C++11 feature list and GCC's,
> picking up the wrong name =)
>
>
> It may be a typo, but the C++ community has clearly adopted the name
> "generalized attributes".

I don't know if I'd say "clearly", considering my initial response to
the wording was "what's a generalized attribute?" ;-)

> I think we should take it and run with it :-)
>
> Reasons to do so:
>
> "C++11 attributes" don't work as a feature name when bringing this to C11 as
> an extension or enabling it in proposed next-generation OpenMP modes. It'd
> be bizarre to diagnose with "C++11 attribute ..." in C11.

There's been some discussion on this, but nothing has actually been
proposed. So I wouldn't start rewording diagnostics based on this just
yet. ;-)

> It feels old keeping the version of introduction in the name. We don't do
> this with other features that have been published, why introduce a
> special-case "C++11 attributes"?

There is something to this argument. An extensive search through our
diagnostics show the usages of C++11 in the diagnostic text are
basically limited to two forms:

XXX is a C++11 extension|feature
XXX does blah blah in C++11

That suggests this warning should read:

"attributes are a C++11 feature"

This has a natural benefit of being similar to the wording we would
use if we ever did bring C++11 attributes to other languages (except
we'd use 'extension' in place of 'feature').

but...

> Leaving the name as just "attributes" is ambiguous in this context because
> users have got used to "attributes" referring to the GNU form.

The other parsing diagnostics refer to C++11 attributes as simply
"attributes", and refer to other attributes by their syntax, so this
change is not consistent with our other diagnostics.

Given that C++11-style attributes are actually standardized, and GNU-
or MS-style attributes are not, what about turning this around a bit:

C++11-style and syntax-agnostic attribute diagnostics get worded as
"attributes", __attribute__-style diagnostics get worded as "GNU
attributes" and __declspec-style diagnostics get worded as "__declspec
attributes".

It's not wholly self-consistent (__declspec vs GNU), but I think
"__attribute__ attributes" looks horrible, and __declspec applies to
more than just Microsoft.

Regardless, the fact that Chandler and Richard are both questioning
the new wording, I think it would make sense to roll this change back
(sorry for the false LGTM on my part!). If we're going to switch the
wording from the established convention within our own diagnostics, it
should have a bit more visible discussion and, more importantly, it
should be consistently applied across related diagnostics.

~Aaron