clang: C++1y deprecated attribute message

[Aaron Ballman]

On Wed, Jan 29, 2014 at 1:07 PM, Richard Smith <metafoo at gmail.com> wrote:
> On Wed Jan 29 2014 at 5:36:44 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> On Wed, Jan 29, 2014 at 2:39 AM, Alp Toker <alp at nuanti.com> wrote:
>> >
>> > On 29/01/2014 06:30, Richard Smith wrote:
>> >
>> > On Tue, Jan 28, 2014 at 4:58 PM, Joseph Mansfield <sftrabbit at gmail.com>
>> > wrote:
>> >>
>> >> The attached patch adds support for deprecation messages in C++1y. This
>> >> is
>> >> my first time hacking with clang so a code review would definitely be
>> >> needed.
>> >>
>> >> As an example of what is now supported:
>> >>
>> >> [[deprecated("use bar instead")]] void foo();
>> >>
>> >> When this function is used, the following warning is emitted:
>> >>
>> >> warning: 'foo' is deprecated: use bar instead
>> >> [-Wdeprecated-declarations]
>> >>   foo();
>> >>   ^
>> >>
>> >> Some tests included.
>> >>
>> >> Known potential issues:
>> >> - Attribute parsing currently doesn't seem to distinguish between C++11
>> >> and C++1y attributes.
>> >> - Maybe errors like [[deprecated("foo", "bar")]] would better report
>> >> "too
>> >> many arguments" than "expected ')'". Not sure about the best way to go
>> >> about
>> >> this.
>> >
>> >
>> > Thanks for the patch! The code generally looks good from a style
>> > perspective, but please start variable names with a capital letter in
>> > new
>> > code (the current codebase is woefully inconsistent, but we're trying to
>> > get
>> > new code to follow the style guide). (The style guide also says to start
>> > functions with a lowercase letter, but we're not doing that for Parse*
>> > because it's such a well-established pattern that the inconsistency
>> > would be
>> > worse than following the style guide. We'll get around to doing a mass
>> > cleanup of this stuff one day...)
>> >
>> > We've been trying to move away from having a collection of different
>> > attribute argument parsing functions, and towards generating the parsing
>> > logic from the Attr.td file -- I'll let Aaron weigh in on whether we
>> > want
>> > something like ParseAttributeWithMessageArg or whether we should
>> > generalize
>> > and reuse ParseGNUAttributeArgs.
>> >
>> >
>> > Indeed, this should work already if you remove the check (ScopeName &&
>> > ScopeName->getName() == "gnu"):
>>
>> Alp hit the nail on the head -- the parsing is already handled for the
>> GNU case, we can simply reuse it.
>
>
> Alp's suggestion needs some refinement -- the standard attributes (other
> than 'deprecated') can't take arguments at all (not even empty parentheses),
> so we'll presumably need some way to encode that in the .td file if we want
> to move away from the hardcoded list.

Ah, so this is a parsing issue as well then.

Perhaps we could use another meta-spelling: StdCXX that expands to:
CXX11<"", name> and CXX11<"std", name>?

>>
>> I'm not certain we need the IsBuiltInOrStandardCXX11Attribute function
>> any longer -- I think this should be declarative-based and handled as
>> part of sema, not parsing (in terms of complaining about repeated
>> attributes) since it really depends on the attribute, not the
>> spelling. Not that it would block this patch.
>>
>> The new spelling does not appear in Attr.td, which is the only thing I
>> really see missing.
>
>
> It's already there and working -- this patch is only adding support for
> doing something with the attribute's argument.

It has no std scope spelling, which I presume is problematic?

~Aaron