From: Aaron Ballman Date: Tue, 13 Apr 2021 10:40:42 +0000 (-0400) Subject: Require commas between double square bracket attributes. X-Git-Tag: llvmorg-14-init~9770 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5ad15f4d1c6f56d25904265023d123a7d0b9d59d;p=platform%2Fupstream%2Fllvm.git Require commas between double square bracket attributes. Clang currently has a bug where it allows you to write [[foo bar]] and both attributes are silently accepted. This patch corrects the comma parsing rules for such attributes and handles the test case fallout, as a few tests were accidentally doing this. --- diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 4980363..e1d29f5 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -4213,10 +4213,21 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, llvm::SmallDenseMap SeenAttrs; + bool AttrParsed = false; while (Tok.isNot(tok::r_square)) { - // attribute not present - if (TryConsumeToken(tok::comma)) - continue; + if (AttrParsed) { + // If we parsed an attribute, a comma is required before parsing any + // additional attributes. + if (ExpectAndConsume(tok::comma)) { + SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); + continue; + } + AttrParsed = false; + } + + // Eat all remaining superfluous commas before parsing the next attribute. + while (TryConsumeToken(tok::comma)) + ; SourceLocation ScopeLoc, AttrLoc; IdentifierInfo *ScopeName = nullptr, *AttrName = nullptr; @@ -4250,7 +4261,6 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, } bool StandardAttr = IsBuiltInOrStandardCXX11Attribute(AttrName, ScopeName); - bool AttrParsed = false; if (StandardAttr && !SeenAttrs.insert(std::make_pair(AttrName, AttrLoc)).second) @@ -4262,12 +4272,14 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, AttrParsed = ParseCXX11AttributeArgs(AttrName, AttrLoc, attrs, endLoc, ScopeName, ScopeLoc); - if (!AttrParsed) + if (!AttrParsed) { attrs.addNew( AttrName, SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrLoc, AttrLoc), ScopeName, ScopeLoc, nullptr, 0, getLangOpts().CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x); + AttrParsed = true; + } if (TryConsumeToken(tok::ellipsis)) Diag(Tok, diag::err_cxx11_attribute_forbids_ellipsis) diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp index 4591195..982f18f 100644 --- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -std=c++2a -verify %s struct [[nodiscard]] S1 {}; // ok -struct [[nodiscard nodiscard]] S2 {}; // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}} +struct [[nodiscard, nodiscard]] S2 {}; // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}} struct [[nodiscard("Wrong")]] S3 {}; [[nodiscard]] int f(); diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p1.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p1.cpp index 8da2ca7..6c9b8a7 100644 --- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p1.cpp +++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p1.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -Wunused -std=c++1z -verify %s struct [[maybe_unused]] S1 {}; // ok -struct [[maybe_unused maybe_unused]] S2 {}; // expected-error {{attribute 'maybe_unused' cannot appear multiple times in an attribute specifier}} +struct [[maybe_unused, maybe_unused]] S2 {}; // expected-error {{attribute 'maybe_unused' cannot appear multiple times in an attribute specifier}} struct [[maybe_unused("Wrong")]] S3 {}; // expected-error {{'maybe_unused' cannot have an argument list}} diff --git a/clang/test/Parser/c2x-attributes.c b/clang/test/Parser/c2x-attributes.c index 393506e..48bb197 100644 --- a/clang/test/Parser/c2x-attributes.c +++ b/clang/test/Parser/c2x-attributes.c @@ -16,6 +16,13 @@ enum { [[]] Six }; // expected-error {{expected identifier}} // FIXME: this diagnostic can be improved. enum E3 [[]] { Seven }; // expected-error {{expected identifier or '('}} +[[,,,,,]] int Commas1; // ok +[[,, maybe_unused]] int Commas2; // ok +[[maybe_unused,,,]] int Commas3; // ok +[[,,maybe_unused,]] int Commas4; // ok +[[foo bar]] int NoComma; // expected-error {{expected ','}} \ + // expected-warning {{unknown attribute 'foo' ignored}} + struct [[]] S1 { int i [[]]; int [[]] j; @@ -117,8 +124,7 @@ void test_asm(void) { } // Do not allow 'using' to introduce vendor attribute namespaces. -[[using vendor: attr1, attr2]] void f14(void); // expected-error {{expected ']'}} \ - // expected-warning {{unknown attribute 'vendor' ignored}} \ +[[using vendor: attr1, attr2]] void f14(void); // expected-error {{expected ','}} \ // expected-warning {{unknown attribute 'using' ignored}} struct [[]] S4 *s; // expected-error {{an attribute list cannot appear here}} diff --git a/clang/test/Parser/cxx-attributes.cpp b/clang/test/Parser/cxx-attributes.cpp index 53b098b..2b874e4 100644 --- a/clang/test/Parser/cxx-attributes.cpp +++ b/clang/test/Parser/cxx-attributes.cpp @@ -34,3 +34,10 @@ void fn() { int (*__attribute__((attr(i[1]))) pi); // expected-warning{{unknown attribute 'attr' ignored}} pi = &i[0]; } + +[[,,,,,]] int Commas1; // ok +[[,, maybe_unused]] int Commas2; // ok +[[maybe_unused,,,]] int Commas3; // ok +[[,,maybe_unused,]] int Commas4; // ok +[[foo bar]] int NoComma; // expected-error {{expected ','}} \ + // expected-warning {{unknown attribute 'foo' ignored}} diff --git a/clang/test/Parser/pragma-attribute.cpp b/clang/test/Parser/pragma-attribute.cpp index 4644bc1..d51f815 100644 --- a/clang/test/Parser/pragma-attribute.cpp +++ b/clang/test/Parser/pragma-attribute.cpp @@ -165,9 +165,9 @@ _Pragma("clang attribute pop"); #pragma clang attribute push ([[]], apply_to = function) // A noop -#pragma clang attribute push ([[noreturn ""]], apply_to=function) // expected-error {{expected ']'}} +#pragma clang attribute push ([[noreturn ""]], apply_to=function) // expected-error {{expected ','}} #pragma clang attribute pop -#pragma clang attribute push ([[noreturn 42]]) // expected-error {{expected ']'}} expected-error {{expected ','}} +#pragma clang attribute push ([[noreturn 42]]) // expected-error 2 {{expected ','}} #pragma clang attribute push(__attribute__, apply_to=function) // expected-error {{expected '(' after 'attribute'}} #pragma clang attribute push(__attribute__(), apply_to=function) // expected-error {{expected '(' after '('}} diff --git a/clang/test/Sema/c2x-maybe_unused-errors.c b/clang/test/Sema/c2x-maybe_unused-errors.c index 39ec2da..5d3eb4d 100644 --- a/clang/test/Sema/c2x-maybe_unused-errors.c +++ b/clang/test/Sema/c2x-maybe_unused-errors.c @@ -3,7 +3,7 @@ struct [[maybe_unused]] S1 { // ok int a [[maybe_unused]]; }; -struct [[maybe_unused maybe_unused]] S2 { // expected-error {{attribute 'maybe_unused' cannot appear multiple times in an attribute specifier}} +struct [[maybe_unused, maybe_unused]] S2 { // expected-error {{attribute 'maybe_unused' cannot appear multiple times in an attribute specifier}} int a; }; struct [[maybe_unused("Wrong")]] S3 { // expected-error {{'maybe_unused' cannot have an argument list}} diff --git a/clang/test/Sema/c2x-nodiscard.c b/clang/test/Sema/c2x-nodiscard.c index cf1f081..f62ba27 100644 --- a/clang/test/Sema/c2x-nodiscard.c +++ b/clang/test/Sema/c2x-nodiscard.c @@ -3,7 +3,7 @@ struct [[nodiscard]] S1 { // ok int i; }; -struct [[nodiscard nodiscard]] S2 { // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}} +struct [[nodiscard, nodiscard]] S2 { // expected-error {{attribute 'nodiscard' cannot appear multiple times in an attribute specifier}} int i; }; struct [[nodiscard("Wrong")]] S3 { // FIXME: may need an extension warning.