r199053 - Clarify warn_cxx98_compat_attribute diagnostic

[Alp Toker]

On 14/01/2014 13:57, Aaron Ballman wrote:
> 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"

I like this. In this context your suggestion seems clearer than both the 
previous diagnostic and my update to the wording in r199053.

>
> 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.

Let's fix forward. The old diagnostic was obviously problematic but it 
looks like you have a reasonable alternative wording to go with in this 
specific context.

That said, the naming problem for attributes isn't going to go away --  
the desire has been expressed to use generalized attribute syntax in 
OpenMP and as a C11 extension, so we'll have to tackle this face-on 
sooner rather than later.

It'll be sad if we end up with "%select{C++11|C11|OpenMP} attributes" a 
year down the line because decisions were made for the wrong reason 
today and we have to make changes with a view to the entire parser, not 
just individual standards as they stand today.

Alp.



>
> ~Aaron

-- 
http://www.nuanti.com
the browser experts