r194869 - PR8455: Handle an attribute between a goto label and a variable declaration per
Alp Toker
alp at nuanti.com
Fri Nov 15 16:00:29 PST 2013
On 15/11/2013 23:42, Richard Smith wrote:
> On Fri, Nov 15, 2013 at 3:33 PM, Alp Toker <alp at nuanti.com
> <mailto:alp at nuanti.com>> 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
> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
> --
> http://www.nuanti.com
> the browser experts
>
>
--
http://www.nuanti.com
the browser experts
More information about the cfe-commits
mailing list