On Tue Jan 14 2014 at 1:33:44 PM, Sean Silva <[email protected]> wrote:
On Tue, Jan 14, 2014 at 4:29 PM, Sean Silva <[email protected]> wrote: 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 This is the diagnostic form we use when a C++11 feature is used in C++98, not the form we use for -Wc++98-compat (when the feature is used in C++11). For the latter, we almost exclusively use a "<blah> is/are incompatible with C++98" wording. (There are 6 exceptions in 76 diagnostics: 5x"<blah> is/would be/might be <blah> in C++98", 1x"C++98 requires <blah>") So for consistency, this should be phrased as "<blah> attributes are incompatible with C++98", fsvo <blah>. 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". Well: 1) the context is an "incompatible with C++98" diagnostic 2) we really don't need to worry about theoretical future C17 or OpenMP constructs now; we can easily change our diagnostics if we ever support those things 3) we have the '[[' in the snippet 4) a %select is *better* than a single fixed string if it makes the diagnostic clearer I'm fine with "'[[...]]' attributes" or similar, although it seems a little redundant given (3). "C++ attributes" or "C++11 attributes" work for me. -- Sean Silva -- Sean Silva Alp. ~Aaron -- http://www.nuanti.com the browser experts _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
