r194869 - PR8455: Handle an attribute between a goto label and a variable declaration per
Alp Toker
alp at nuanti.com
Fri Nov 15 16:07:50 PST 2013
On 16/11/2013 00:00, Alp Toker wrote:
>
> 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.
In fact, this applies not just to Microsoft-style attributes but also
Objective-C messages using square brackets.
Tentative parse was the only solution to handle this correctly in
Objective-C++ using GNU labeled statements with C++0x attributes when
the patch was written three years ago, and although the standards
weren't final yet I don't think that's changed.
Have you actually tested corner cases like this?
Alp.
>
> 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