r194869 - PR8455: Handle an attribute between a goto label and a variable declaration per

Richard Smith richard at metafoo.co.uk
Fri Nov 15 15:42:37 PST 2013


On Fri, Nov 15, 2013 at 3:33 PM, Alp Toker <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.


> 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
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131115/b06813ab/attachment.html>


More information about the cfe-commits mailing list