clang: C++1y deprecated attribute message

[Richard Smith]

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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140129/8dd73a91/attachment.html>