clang: C++1y deprecated attribute message

[Richard Smith]

On Wed Jan 29 2014 at 11:22:29 AM, Aaron Ballman <aaron at aaronballman.com>
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. :-(


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".)

We have (non-normative) encouragement to put non-standard attributes in our
own attribute namespace (see the note in 7.6.1/3).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140129/7bb5c2dd/attachment.html>