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.


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.

Alp.




~Aaron

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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to