clang: C++1y deprecated attribute message

[Alp Toker]

On 29/01/2014 06:30, Richard Smith wrote:
> On Tue, Jan 28, 2014 at 4:58 PM, Joseph Mansfield <sftrabbit at gmail.com 
> <mailto: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"):

|$ cat attr.cpp||
||[[deprecated("reason")]] int f();||
||
||$ clang -cc1 -std=c++11 -ast-print attr.cpp ||
||int f() [[deprecated("reason")]];|

No need for custom parsing.

Alp.


>
> +  if (Tok.isNot(tok::string_literal)) {
> +    Diag(Tok, diag::err_expected_string_literal)
> +      << /*Source='attribute'*/2
> +      << AttrName.getName();
> +    SkipUntil(tok::r_paren, StopAtSemi);
> +    return;
> +  }
>
> This check isn't quite right: there are five different tokens that can 
> start a string literal (string_literal, wide_string_literal, 
> utf8_string_literal, ...) -- use isTokenStringLiteral() instead.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140129/a367e890/attachment.html>