<div>On Wed Jan 29 2014 at 11:22:29 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Wed, Jan 29, 2014 at 2:04 PM, Richard Smith <<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a>> wrote:<br>
> On Wed Jan 29 2014 at 10:35:46 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> On Wed, Jan 29, 2014 at 1:07 PM, Richard Smith <<a href="mailto:metafoo@gmail.com" target="_blank">metafoo@gmail.com</a>> wrote:<br>
>> > On Wed Jan 29 2014 at 5:36:44 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Wed, Jan 29, 2014 at 2:39 AM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>> wrote:<br>
>> >> ><br>
>> >> > On 29/01/2014 06:30, Richard Smith wrote:<br>
>> >> ><br>
>> >> > On Tue, Jan 28, 2014 at 4:58 PM, Joseph Mansfield<br>
>> >> > <<a href="mailto:sftrabbit@gmail.com" target="_blank">sftrabbit@gmail.com</a>><br>
>> >> > wrote:<br>
>> >> >><br>
>> >> >> The attached patch adds support for deprecation messages in C++1y.<br>
>> >> >> This<br>
>> >> >> is<br>
>> >> >> my first time hacking with clang so a code review would definitely<br>
>> >> >> be<br>
>> >> >> needed.<br>
>> >> >><br>
>> >> >> As an example of what is now supported:<br>
>> >> >><br>
>> >> >> [[deprecated("use bar instead")]] void foo();<br>
>> >> >><br>
>> >> >> When this function is used, the following warning is emitted:<br>
>> >> >><br>
>> >> >> warning: 'foo' is deprecated: use bar instead<br>
>> >> >> [-Wdeprecated-declarations]<br>
>> >> >>   foo();<br>
>> >> >>   ^<br>
>> >> >><br>
>> >> >> Some tests included.<br>
>> >> >><br>
>> >> >> Known potential issues:<br>
>> >> >> - Attribute parsing currently doesn't seem to distinguish between<br>
>> >> >> C++11<br>
>> >> >> and C++1y attributes.<br>
>> >> >> - Maybe errors like [[deprecated("foo", "bar")]] would better report<br>
>> >> >> "too<br>
>> >> >> many arguments" than "expected ')'". Not sure about the best way to<br>
>> >> >> go<br>
>> >> >> about<br>
>> >> >> this.<br>
>> >> ><br>
>> >> ><br>
>> >> > Thanks for the patch! The code generally looks good from a style<br>
>> >> > perspective, but please start variable names with a capital letter in<br>
>> >> > new<br>
>> >> > code (the current codebase is woefully inconsistent, but we're trying<br>
>> >> > to<br>
>> >> > get<br>
>> >> > new code to follow the style guide). (The style guide also says to<br>
>> >> > start<br>
>> >> > functions with a lowercase letter, but we're not doing that for<br>
>> >> > Parse*<br>
>> >> > because it's such a well-established pattern that the inconsistency<br>
>> >> > would be<br>
>> >> > worse than following the style guide. We'll get around to doing a<br>
>> >> > mass<br>
>> >> > cleanup of this stuff one day...)<br>
>> >> ><br>
>> >> > We've been trying to move away from having a collection of different<br>
>> >> > attribute argument parsing functions, and towards generating the<br>
>> >> > parsing<br>
>> >> > logic from the Attr.td file -- I'll let Aaron weigh in on whether we<br>
>> >> > want<br>
>> >> > something like ParseAttributeWithMessageArg or whether we should<br>
>> >> > generalize<br>
>> >> > and reuse ParseGNUAttributeArgs.<br>
>> >> ><br>
>> >> ><br>
>> >> > Indeed, this should work already if you remove the check (ScopeName<br>
>> >> > &&<br>
>> >> > ScopeName->getName() == "gnu"):<br>
>> >><br>
>> >> Alp hit the nail on the head -- the parsing is already handled for the<br>
>> >> GNU case, we can simply reuse it.<br>
>> ><br>
>> ><br>
>> > Alp's suggestion needs some refinement -- the standard attributes (other<br>
>> > than 'deprecated') can't take arguments at all (not even empty<br>
>> > parentheses),<br>
>> > so we'll presumably need some way to encode that in the .td file if we<br>
>> > want<br>
>> > to move away from the hardcoded list.<br>
>><br>
>> Ah, so this is a parsing issue as well then.<br>
>><br>
>> Perhaps we could use another meta-spelling: StdCXX that expands to:<br>
>> CXX11<"", name> and CXX11<"std", name>?<br>
>><br>
>> >><br>
>> >> I'm not certain we need the IsBuiltInOrStandardCXX11Attrib<u></u>ute function<br>
>> >> any longer -- I think this should be declarative-based and handled as<br>
>> >> part of sema, not parsing (in terms of complaining about repeated<br>
>> >> attributes) since it really depends on the attribute, not the<br>
>> >> spelling. Not that it would block this patch.<br>
>> >><br>
>> >> The new spelling does not appear in Attr.td, which is the only thing I<br>
>> >> really see missing.<br>
>> ><br>
>> ><br>
>> > It's already there and working -- this patch is only adding support for<br>
>> > doing something with the attribute's argument.<br>
>><br>
>> It has no std scope spelling, which I presume is problematic?<br>
><br>
><br>
> I don't see any point in the 'std' attribute namespace, and looking through<br>
> svn history, I can't see any justification being provided when it was added.<br>
> Just remove those spellings entirely?<br>
<br>
I see nothing in the standard which suggests these should be in the<br>
std:: namespace, so I'm in favor of removing those spellings.<br>
<br>
Then again, I also don't see anything in the standard claiming that<br>
unscoped attributes are reserved which makes me fearful for naming<br>
collisions with different semantics between implementations. :-(</blockquote><div><br></div><div>They're intended to be available for future standardization, with (IIRC) the intent that attributes from C++1y are a conforming extension in C++11, so they can't /really/ be reserved and still support that use case. (They could say something like "reserved for future standardization", but that doesn't quite capture the intent as I understand it -- they really want "other unscoped attributes are conditionally-supported, with semantics as specified in some unknown future C++ standard".)</div>
<div><br></div><div>We have (non-normative) encouragement to put non-standard attributes in our own attribute namespace (see the note in 7.6.1/3).</div>