On Thu, Apr 15, 2021 at 12:15 AM <douglas.y...@sony.com> wrote: > > Hi Aaron, > > Your change is causing the compiler to go into an infinite loop in one of our > internal tests. I have put details up in PR49966, can you please take a look?
Absolutely, thank you for the report! ~Aaron > > Douglas Yung > > -----Original Message----- > From: cfe-commits <cfe-commits-boun...@lists.llvm.org> On Behalf Of Aaron > Ballman via cfe-commits > Sent: Tuesday, April 13, 2021 3:43 > To: cfe-commits@lists.llvm.org > Subject: [clang] 5ad15f4 - Require commas between double square bracket > attributes. > > > Author: Aaron Ballman > Date: 2021-04-13T06:43:01-04:00 > New Revision: 5ad15f4d1c6f56d25904265023d123a7d0b9d59d > > URL: > https://github.com/llvm/llvm-project/commit/5ad15f4d1c6f56d25904265023d123a7d0b9d59d > DIFF: > https://github.com/llvm/llvm-project/commit/5ad15f4d1c6f56d25904265023d123a7d0b9d59d.diff > > LOG: 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. > > Added: > > > Modified: > clang/lib/Parse/ParseDeclCXX.cpp > clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p1.cpp > clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p1.cpp > clang/test/Parser/c2x-attributes.c > clang/test/Parser/cxx-attributes.cpp > clang/test/Parser/pragma-attribute.cpp > clang/test/Sema/c2x-maybe_unused-errors.c > clang/test/Sema/c2x-nodiscard.c > > Removed: > > > > ################################################################################ > diff --git a/clang/lib/Parse/ParseDeclCXX.cpp > b/clang/lib/Parse/ParseDeclCXX.cpp > index 498036368c19..e1d29f555f42 100644 > --- a/clang/lib/Parse/ParseDeclCXX.cpp > +++ b/clang/lib/Parse/ParseDeclCXX.cpp > @@ -4213,10 +4213,21 @@ void > Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, > > llvm::SmallDenseMap<IdentifierInfo*, SourceLocation, 4> 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 45911958af74..982f18f1e8cd 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 8da2ca7d6d86..6c9b8a75cb04 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 393506e867fe..48bb19707dd1 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 53b098b6260a..2b874e4aca7f 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 4644bc18359e..d51f8159c263 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 39ec2da9a152..5d3eb4d47a6e 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 cf1f0818419a..f62ba27c12a1 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. > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
