clang: C++1y deprecated attribute message

[Alp Toker]

On 29/01/2014 19:22, Aaron Ballman wrote:
> On Wed, Jan 29, 2014 at 2:04 PM, Richard Smith <metafoo at gmail.com> wrote:
>> On Wed Jan 29 2014 at 10:35:46 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>>> 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?
>>
>> I don't see any point in the 'std' attribute namespace, and looking through
>> svn history, I can't see any justification being provided when it was added.
>> Just remove those spellings entirely?
> I see nothing in the standard which suggests these should be in the
> std:: namespace, so I'm in favor of removing those spellings.
>
> Then again, I also don't see anything in the standard claiming that
> unscoped attributes are reserved which makes me fearful for naming
> collisions with different semantics between implementations. :-(

Slightly OT story on the side...

Back in the day with an early interpretation of C++0x attributes we had 
in-house, we put attributes in their own IDNS and used ordinary 
namespace lookup so they could be brought into scope with a 'using', 
aliased and even overloaded. It worked quite nicely and though I guess 
the ship has set sail I'm always wondering if we can pick that up again 
somehow in a way that's compatible with the current spec.

Alp.



>
> ~Aaron

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