<div>On Tue Jan 14 2014 at 1:33:44 PM, Sean Silva <<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>> wrote:</div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div>On Tue, Jan 14, 2014 at 4:29 PM, Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div><br><br><div><div><div>On Tue, Jan 14, 2014 at 9:10 AM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
On 14/01/2014 13:57, Aaron Ballman wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Tue, Jan 14, 2014 at 3:25 AM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>> wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 14/01/2014 03:10, Richard Smith wrote:<br>
<br>
On Mon, Jan 13, 2014 at 6:37 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Jan 13, 2014 at 9:24 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>><br>
wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Sun, Jan 12, 2014 at 11:14 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
wrote:<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
After doing a bit more research and discussion off-list, I think<br>
"generalized attribute" is acceptable. So patch LGTM as-is.<br>
</blockquote>
<br>
Really? I wouldn't expect someone seeing this diagnostic to understand<br>
that<br>
"generalized attribute" means C++11 attributes (it's a really weird<br>
term,<br>
since they're not a generalization of anything). This isn't an official<br>
name<br>
for them, and doesn't distinguish them from the other attribute syntaxes<br>
we<br>
support. Given that this is a diagnostic about compatibility with C++98,<br>
"C++11 attributes" seems like the clearest way of expressing this.<br>
</blockquote>
As Alp had pointed out, we document the name as "generalized<br>
attribute" in our feature support documentation,<br>
</blockquote>
<br>
You're right, we did. I just fixed that.<br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
and it's the original name of the feature.<br>
</blockquote>
<br>
[citation needed]<br>
<br>
The proposal calls them "General Attributes for C++" (and all previous<br>
revisions of it did the same); the word "Generalized" seems to have been<br>
accidentally transferred from "Generalized constant expressions" in the GCC<br>
list, and we inherited that mistake when we sync'd our list with theirs in<br>
r142015. The paper and C++ standard both call them simply "attributes".<br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, a quick google search of "generalized<br>
attribute" yielded more results than "C++11 attribute" did (not saying<br>
this was particularly scientific). So that's why I gave the LGTM on<br>
the term.<br>
</blockquote>
<br>
Hah, it seems that lots of people copied our C++11 feature list and GCC's,<br>
picking up the wrong name =)<br>
<br>
<br>
It may be a typo, but the C++ community has clearly adopted the name<br>
"generalized attributes".<br>
</blockquote>
I don't know if I'd say "clearly", considering my initial response to<br>
the wording was "what's a generalized attribute?" ;-)<br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I think we should take it and run with it :-)<br>
<br>
Reasons to do so:<br>
<br>
"C++11 attributes" don't work as a feature name when bringing this to C11 as<br>
an extension or enabling it in proposed next-generation OpenMP modes. It'd<br>
be bizarre to diagnose with "C++11 attribute ..." in C11.<br>
</blockquote>
There's been some discussion on this, but nothing has actually been<br>
proposed. So I wouldn't start rewording diagnostics based on this just<br>
yet. ;-)<br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It feels old keeping the version of introduction in the name. We don't do<br>
this with other features that have been published, why introduce a<br>
special-case "C++11 attributes"?<br>
</blockquote>
There is something to this argument. An extensive search through our<br>
diagnostics show the usages of C++11 in the diagnostic text are<br>
basically limited to two forms:<br>
<br>
XXX is a C++11 extension|feature<br></blockquote></div></div></blockquote></div></div></div></div></div></blockquote></div></div></div></blockquote><div><br></div><div>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>")</div>
<div><br></div><div>So for consistency, this should be phrased as "<blah> attributes are incompatible with C++98", fsvo <blah>.</div><div><br></div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
XXX does blah blah in C++11<br>
<br>
That suggests this warning should read:<br>
<br>
"attributes are a C++11 feature"<br>
</blockquote>
<br></div></div>
I like this. In this context your suggestion seems clearer than both the previous diagnostic and my update to the wording in r199053.<div><br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
This has a natural benefit of being similar to the wording we would<br>
use if we ever did bring C++11 attributes to other languages (except<br>
we'd use 'extension' in place of 'feature').<br>
<br>
but...<br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Leaving the name as just "attributes" is ambiguous in this context because<br>
users have got used to "attributes" referring to the GNU form.<br>
</blockquote>
The other parsing diagnostics refer to C++11 attributes as simply<br>
"attributes", and refer to other attributes by their syntax, so this<br>
change is not consistent with our other diagnostics.<br>
<br>
Given that C++11-style attributes are actually standardized, and GNU-<br>
or MS-style attributes are not, what about turning this around a bit:<br>
<br>
C++11-style and syntax-agnostic attribute diagnostics get worded as<br>
"attributes", __attribute__-style diagnostics get worded as "GNU<br>
attributes" and __declspec-style diagnostics get worded as "__declspec<br>
attributes".<br>
<br>
It's not wholly self-consistent (__declspec vs GNU), but I think<br>
"__attribute__ attributes" looks horrible, and __declspec applies to<br>
more than just Microsoft.<br>
<br>
Regardless, the fact that Chandler and Richard are both questioning<br>
the new wording, I think it would make sense to roll this change back<br>
(sorry for the false LGTM on my part!). If we're going to switch the<br>
wording from the established convention within our own diagnostics, it<br>
should have a bit more visible discussion and, more importantly, it<br>
should be consistently applied across related diagnostics.<br>
</blockquote>
<br></div>
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.<br>
<br>
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.<br>
<br>
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.</blockquote>
<div><br></div></div></div><div>WTF just say "double-bracket attributes" and be done with it.</div></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div><div><div>Actually, some people use the term "brackets" to collectively refer to ( [ { ("round brackets", "square brackets", "curly brackets", respectively). Better to just say "`[[` attributes". </div>
</div></div></div></blockquote><div><br></div><div>Well:</div><div>1) the context is an "incompatible with C++98" diagnostic</div><div>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</div>
<div>3) we have the '[[' in the snippet</div><div>4) a %select is *better* than a single fixed string if it makes the diagnostic clearer</div><div><br></div><div>I'm fine with "'[[...]]' attributes" or similar, although it seems a little redundant given (3). "C++ attributes" or "C++11 attributes" work for me.</div>
<div> </div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>-- Sean Silva</div></div></div></div><div dir="ltr"><div><div><div><br></div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div>
<span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><div><div> </div><blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><font color="#888888"><br>
<br>
Alp.</font></span><div><br>
<br>
<br>
<br>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
~Aaron<br>
</blockquote>
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br></div><div><div>
______________________________<u></u><u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailm<u></u>an/listinfo/cfe-commits</a><br>
</div></div></blockquote></div></div><br></div></div>
</blockquote></div></div></div></blockquote>