On Fri, Nov 15, 2013 at 3:33 PM, Alp Toker <[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. > 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] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > -- > http://www.nuanti.com > the browser experts > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
