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