<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 28, 2014 at 4:58 PM, Joseph Mansfield <span dir="ltr"><<a href="mailto:sftrabbit@gmail.com" target="_blank">sftrabbit@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">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.<div>

<br></div><div>As an example of what is now supported:</div>

<div><br></div><div><font face="courier new, monospace">[[deprecated("use bar instead")]] void foo();</font></div><div><br></div><div>When this function is used, the following warning is emitted:</div><div><br>


</div>
<div><div><font face="courier new, monospace">warning: 'foo' is deprecated: use bar instead [-Wdeprecated-declarations]</font></div><div><font face="courier new, monospace">  foo();</font></div><div><font face="courier new, monospace">  ^</font></div>



</div><div><br></div><div>Some tests included.</div><div><br></div><div>Known potential issues:</div><div>- Attribute parsing currently doesn't seem to distinguish between C++11 and C++1y attributes.</div><div>- Maybe errors like [[deprecated("foo", "bar")]] would better report "too many arguments" than "expected ')'". Not sure about the best way to go about this.</div>

</div></blockquote><div><br></div><div>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...)</div>
<div><br></div><div>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.</div>

<div><br></div><div><div>+  if (Tok.isNot(tok::string_literal)) {</div><div>+    Diag(Tok, diag::err_expected_string_literal)</div><div>+      << /*Source='attribute'*/2</div><div>+      << AttrName.getName();</div>

<div>+    SkipUntil(tok::r_paren, StopAtSemi);</div><div>+    return;</div><div>+  }</div></div><div><br></div><div>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.</div>

</div></div></div>