r199053 - Clarify warn_cxx98_compat_attribute diagnostic

[Sean Silva]

On Tue, Jan 14, 2014 at 4:29 PM, Sean Silva <silvas at purdue.edu> wrote:

>
>
>
> On Tue, Jan 14, 2014 at 9:10 AM, Alp Toker <alp at nuanti.com> wrote:
>
>>
>> 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.
>
>
> WTF just say "double-bracket attributes" and be done with it.
>

Actually, some people use the term "brackets" to collectively refer to ( [
{ ("round brackets", "square brackets", "curly brackets", respectively).
Better to just say "`[[` attributes".

-- Sean Silva


> -- Sean Silva
>
>
>>
>>
>> Alp.
>>
>>
>>
>>
>>
>>> ~Aaron
>>>
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140114/e19f43fb/attachment.html>