[clang] 5ad15f4 - Require commas between double square bracket attributes.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 15 04:10:00 PDT 2021
On Thu, Apr 15, 2021 at 12:15 AM <douglas.yung at 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-bounces at lists.llvm.org> On Behalf Of Aaron Ballman via cfe-commits
> Sent: Tuesday, April 13, 2021 3:43
> To: cfe-commits at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list