<div dir="ltr">On Fri, Nov 15, 2013 at 3:33 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello Richard,<br>
<br>
For the record I had a patch up for review to fix this back from 2010, not sure if it ever reached the list:<br>
<br>
<a href="http://llvm.org/bugs/show_bug.cgi?id=8455" target="_blank">http://llvm.org/bugs/show_bug.<u></u>cgi?id=8455</a><br>
<br>
Just to compare notes, my approach was to use a TentativeParsingAction to handle some of the corner cases.<br></blockquote><div><br></div><div>Aaron's original patch went in that direction, but got reworked to avoid the cost of a tentative parse.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Alp.<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<br>
On 15/11/2013 22:45, Richard Smith wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rsmith<br>
Date: Fri Nov 15 16:45:29 2013<br>
New Revision: 194869<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=194869&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=194869&view=rev</a><br>
Log:<br>
PR8455: Handle an attribute between a goto label and a variable declaration per<br>
the GNU documentation: the attribute only appertains to the label if it is<br>
followed by a semicolon. Based on a patch by Aaron Ballman!<br>
<br>
Modified:<br>
cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp<br>
cfe/trunk/test/Misc/ast-dump-<u></u>attr.cpp<br>
cfe/trunk/test/Sema/warn-<u></u>unused-label.c<br>
cfe/trunk/test/SemaCXX/warn-<u></u>unused-variables.cpp<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=194869&r1=194868&r2=194869&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/Parse/<u></u>ParseStmt.cpp?rev=194869&r1=<u></u>194868&r2=194869&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseStmt.<u></u>cpp Fri Nov 15 16:45:29 2013<br>
@@ -516,11 +516,40 @@ StmtResult Parser::ParseLabeledStatement<br>
// identifier ':' statement<br>
SourceLocation ColonLoc = ConsumeToken();<br>
- // Read label attributes, if present. attrs will contain both C++11 and GNU<br>
- // attributes (if present) after this point.<br>
- MaybeParseGNUAttributes(attrs)<u></u>;<br>
+ // Read label attributes, if present.<br>
+ StmtResult SubStmt;<br>
+ if (Tok.is(tok::kw___attribute)) {<br>
+ ParsedAttributesWithRange TempAttrs(AttrFactory);<br>
+ ParseGNUAttributes(TempAttrs);<br>
+<br>
+ // In C++, GNU attributes only apply to the label if they are followed by a<br>
+ // semicolon, to disambiguate label attributes from attributes on a labeled<br>
+ // declaration.<br>
+ //<br>
+ // This doesn't quite match what GCC does; if the attribute list is empty<br>
+ // and followed by a semicolon, GCC will reject (it appears to parse the<br>
+ // attributes as part of a statement in that case). That looks like a bug.<br>
+ if (!getLangOpts().CPlusPlus || Tok.is(tok::semi))<br>
+ attrs.takeAllFrom(TempAttrs);<br>
+ else if (isDeclarationStatement()) {<br>
+ StmtVector Stmts;<br>
+ // FIXME: We should do this whether or not we have a declaration<br>
+ // statement, but that doesn't work correctly (because ProhibitAttributes<br>
+ // can't handle GNU attributes), so only call it in the one case where<br>
+ // GNU attributes are allowed.<br>
+ SubStmt = ParseStatementOrDeclarationAft<u></u>erAttributes(<br>
+ Stmts, /*OnlyStmts*/ true, 0, TempAttrs);<br>
+ if (!TempAttrs.empty() && !SubStmt.isInvalid())<br>
+ SubStmt = Actions.ProcessStmtAttributes(<br>
+ SubStmt.get(), TempAttrs.getList(), TempAttrs.Range);<br>
+ } else {<br>
+ Diag(Tok, diag::err_expected_semi_after) << "__attribute__";<br>
+ }<br>
+ }<br>
- StmtResult SubStmt(ParseStatement());<br>
+ // If we've not parsed a statement yet, parse one now.<br>
+ if (!SubStmt.isInvalid() && !SubStmt.isUsable())<br>
+ SubStmt = ParseStatement();<br>
// Broken substmt shouldn't prevent the label from being added to the AST.<br>
if (SubStmt.isInvalid())<br>
@@ -557,7 +586,7 @@ StmtResult Parser::ParseCaseStatement(bo<br>
// out of stack space in our recursive descent parser. As a special case,<br>
// flatten this recursion into an iterative loop. This is complex and gross,<br>
// but all the grossness is constrained to ParseCaseStatement (and some<br>
- // wierdness in the actions), so this is just local grossness :).<br>
+ // weirdness in the actions), so this is just local grossness :).<br>
// TopLevelCase - This is the highest level we have parsed. 'case 1' in the<br>
// example above.<br>
<br>
Modified: cfe/trunk/test/Misc/ast-dump-<u></u>attr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-attr.cpp?rev=194869&r1=194868&r2=194869&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/test/Misc/<u></u>ast-dump-attr.cpp?rev=194869&<u></u>r1=194868&r2=194869&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/test/Misc/ast-dump-<u></u>attr.cpp (original)<br>
+++ cfe/trunk/test/Misc/ast-dump-<u></u>attr.cpp Fri Nov 15 16:45:29 2013<br>
@@ -95,3 +95,19 @@ void *TestVariadicUnsigned1(int) __attri<br>
void *TestVariadicUnsigned2(int, int) __attribute__((alloc_size(1,2)<u></u>));<br>
// CHECK: FunctionDecl{{.*}}<u></u>TestVariadicUnsigned2<br>
// CHECK: AllocSizeAttr{{.*}} 0 1<br>
+<br>
+void TestLabel() {<br>
+L: __attribute__((unused)) int i;<br>
+// CHECK: LabelStmt{{.*}}'L'<br>
+// CHECK: VarDecl{{.*}}i 'int'<br>
+// CHECK-NEXT: UnusedAttr{{.*}}<br>
+<br>
+M: __attribute(()) int j;<br>
+// CHECK: LabelStmt {{.*}} 'M'<br>
+// CHECK-NEXT: DeclStmt<br>
+// CHECK-NEXT: VarDecl {{.*}} j 'int'<br>
+<br>
+N: __attribute(()) ;<br>
+// CHECK: LabelStmt {{.*}} 'N'<br>
+// CHECK-NEXT: NullStmt<br>
+}<br>
<br>
Modified: cfe/trunk/test/Sema/warn-<u></u>unused-label.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-unused-label.c?rev=194869&r1=194868&r2=194869&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/test/Sema/<u></u>warn-unused-label.c?rev=<u></u>194869&r1=194868&r2=194869&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/test/Sema/warn-<u></u>unused-label.c (original)<br>
+++ cfe/trunk/test/Sema/warn-<u></u>unused-label.c Fri Nov 15 16:45:29 2013<br>
@@ -9,3 +9,7 @@ void f() {<br>
goto d;<br>
return;<br>
}<br>
+<br>
+void PR8455() {<br>
+ L: __attribute__((unused)) return; // ok, no semicolon required<br>
+}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-<u></u>unused-variables.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-variables.cpp?rev=194869&r1=194868&r2=194869&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/test/<u></u>SemaCXX/warn-unused-variables.<u></u>cpp?rev=194869&r1=194868&r2=<u></u>194869&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/test/SemaCXX/warn-<u></u>unused-variables.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-<u></u>unused-variables.cpp Fri Nov 15 16:45:29 2013<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -verify %s<br>
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -verify %s<br>
template<typename T> void f() {<br>
T t;<br>
t = 17;<br>
@@ -128,3 +128,26 @@ namespace ctor_with_cleanups {<br>
}<br>
#include "Inputs/warn-unused-variables.<u></u>h"<br>
+<br>
+namespace PR8455 {<br>
+ void f() {<br>
+ A: // expected-warning {{unused label 'A'}}<br>
+ __attribute__((unused)) int i; // attribute applies to variable<br>
+ B: // attribute applies to label<br>
+ __attribute__((unused)); int j; // expected-warning {{unused variable 'j'}}<br>
+ }<br>
+<br>
+ void g() {<br>
+ C: // unused label 'C' will not appear here because an error occurs<br>
+ __attribute__((unused))<br>
+ #pragma weak unused_local_static // expected-error {{expected ';' after __attribute__}}<br>
+ ;<br>
+ }<br>
+<br>
+ void h() {<br>
+ D: // expected-warning {{unused label 'D'}}<br>
+ #pragma weak unused_local_static<br>
+ __attribute__((unused)) // expected-warning {{declaration does not declare anything}}<br>
+ ;<br>
+ }<br>
+}<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br></div></div><span class="HOEnZb"><font color="#888888">
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br></div></div>