On Tue, Jan 14, 2014 at 9:10 AM, Alp Toker <[email protected]> wrote: > > On 14/01/2014 13:57, Aaron Ballman wrote: >> >> On Tue, Jan 14, 2014 at 3:25 AM, Alp Toker <[email protected]> wrote: >>> >>> On 14/01/2014 03:10, Richard Smith wrote: >>> >>> On Mon, Jan 13, 2014 at 6:37 PM, Aaron Ballman <[email protected]> >>> wrote: >>>> >>>> On Mon, Jan 13, 2014 at 9:24 PM, Richard Smith <[email protected]> >>>> wrote: >>>>> >>>>> On Sun, Jan 12, 2014 at 11:14 AM, Aaron Ballman >>>>> <[email protected]> >>>>> 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.
Great! >> >> 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. I'm fine with that. Also, to be clear: the "false LGTM" wasn't that you did anything wrong, it was for my waffling after further discussion and thought. ;-) > 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. Yeah, that's the unfortunate reality of any overloaded term. Thankfully, in most cases, the overload is only one or two uses. Here it's three or four (so far!). ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
