r199053 - Clarify warn_cxx98_compat_attribute diagnostic

Alp Toker alp at nuanti.com
Tue Jan 14 15:29:48 PST 2014


On 14/01/2014 21:33, Sean Silva wrote:
>
>
>
> On Tue, Jan 14, 2014 at 4:29 PM, Sean Silva <silvas at purdue.edu 
> <mailto:silvas at purdue.edu>> wrote:
>
>
>
>
>     On Tue, Jan 14, 2014 at 9:10 AM, Alp Toker <alp at nuanti.com
>     <mailto: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
>             <mailto: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 <mailto:aaron at aaronballman.com>>
>                 wrote:
>
>                     On Mon, Jan 13, 2014 at 9:24 PM, Richard Smith
>                     <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>>
>                     wrote:
>
>                         On Sun, Jan 12, 2014 at 11:14 AM, Aaron
>                         Ballman <aaron at aaronballman.com
>                         <mailto: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".

Hi Sean,

Just saw this. The generalized name will need a -W flag so it needs to 
be [a-z] keeping in mind also that -W(no-)attributes is taken:

[cfe-dev] -Wattributes versus -Wunknown-attributes)
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-June/015412.html

Alp.

>
> -- Sean Silva
>
>
>     -- Sean Silva
>
>
>
>         Alp.
>
>
>
>
>
>             ~Aaron
>
>
>         -- 
>         http://www.nuanti.com
>         the browser experts
>
>         _______________________________________________
>         cfe-commits mailing list
>         cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>         http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>

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




More information about the cfe-commits mailing list