| Message ID | 20201106190611.911508-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: DR 1914 - Allow duplicate standard attributes. | expand |
On 11/6/20 2:06 PM, Marek Polacek wrote: > Following Joseph's change for C to allow duplicate C2x standard attributes > <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, > this patch does a similar thing for C++. This is DR 1914, to be resolved by > <wg21.link/p2156>, which is not part of the standard yet, but has a wide > support so look like a shoo-in. Some duplications still produce warnings; > I didn't change that because a warning might be desirable. What's the rationale for warning about some and not others? > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > gcc/c-family/ChangeLog: > > DR 1914 > * c-common.c (attribute_fallthrough_p): Update comment. > > gcc/cp/ChangeLog: > > DR 1914 > * parser.c (cp_parser_check_std_attribute): Remove. > (cp_parser_std_attribute_list): Don't call it. > > gcc/testsuite/ChangeLog: > > DR 1914 > * g++.dg/cpp0x/gen-attrs-60.C: Remove dg-error. > * g++.dg/cpp2a/nodiscard-once.C: Likewise. > * g++.dg/cpp1y/attr-deprecated-2.C: Likewise. > * g++.dg/cpp0x/gen-attrs-72.C: New test. > --- > gcc/c-family/c-common.c | 3 +- > gcc/cp/parser.c | 27 ------------ > gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C | 2 +- > gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C | 42 +++++++++++++++++++ > .../g++.dg/cpp1y/attr-deprecated-2.C | 2 +- > gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C | 2 +- > 6 files changed, 47 insertions(+), 31 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index 232a4797c09..03d8fec9936 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -5751,7 +5751,8 @@ attribute_fallthrough_p (tree attr) > tree t = lookup_attribute ("fallthrough", attr); > if (t == NULL_TREE) > return false; > - /* This attribute shall appear at most once in each attribute-list. */ > + /* It is no longer true that "this attribute shall appear at most once in > + each attribute-list", but we still give a warning. */ > if (lookup_attribute ("fallthrough", TREE_CHAIN (t))) > warning (OPT_Wattributes, "%<fallthrough%> attribute specified multiple " > "times"); > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 6e7b982f073..0567646fbe2 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -27269,32 +27269,6 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) > return attribute; > } > > -/* Check that the attribute ATTRIBUTE appears at most once in the > - attribute-list ATTRIBUTES. This is enforced for noreturn (7.6.3), > - nodiscard, and deprecated (7.6.5). Note that > - carries_dependency (7.6.4) isn't implemented yet in GCC. */ > - > -static void > -cp_parser_check_std_attribute (tree attributes, tree attribute) > -{ > - if (attributes) > - { > - tree name = get_attribute_name (attribute); > - if (is_attribute_p ("noreturn", name) > - && lookup_attribute ("noreturn", attributes)) > - error ("attribute %<noreturn%> can appear at most once " > - "in an attribute-list"); > - else if (is_attribute_p ("deprecated", name) > - && lookup_attribute ("deprecated", attributes)) > - error ("attribute %<deprecated%> can appear at most once " > - "in an attribute-list"); > - else if (is_attribute_p ("nodiscard", name) > - && lookup_attribute ("nodiscard", attributes)) > - error ("attribute %<nodiscard%> can appear at most once " > - "in an attribute-list"); > - } > -} > - > /* Parse a list of standard C++-11 attributes. > > attribute-list: > @@ -27317,7 +27291,6 @@ cp_parser_std_attribute_list (cp_parser *parser, tree attr_ns) > break; > if (attribute != NULL_TREE) > { > - cp_parser_check_std_attribute (attributes, attribute); > TREE_CHAIN (attribute) = attributes; > attributes = attribute; > } > diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C > index cb0c31ec63f..4e905c394ca 100644 > --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C > +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C > @@ -1,4 +1,4 @@ > // PR c++/60365 > // { dg-do compile { target c++11 } } > > -void func [[noreturn, noreturn]] (); // { dg-error "at most once" } > +void func [[noreturn, noreturn]] (); > diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C > new file mode 100644 > index 00000000000..0cb874b6191 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C > @@ -0,0 +1,42 @@ > +// DR 1914 - Duplicate standard attributes > +// { dg-do compile { target c++11 } } > + > +[[noreturn, noreturn]] void fn0(); > +[[noreturn]] [[noreturn]] void fn1(); > +[[deprecated, deprecated]] void fn2(); > +[[deprecated]] [[deprecated]] void fn3(); > +[[maybe_unused]] [[maybe_unused]] int fn4(); > +[[maybe_unused, maybe_unused]] int fn5(); > +[[nodiscard]] [[nodiscard]] int fn6(); > +[[nodiscard, nodiscard]] int fn7(); > + > +struct E { }; > +struct A { > + [[no_unique_address]] [[no_unique_address]] E e; > +}; > +struct B { > + [[no_unique_address, no_unique_address]] E e; > +}; > + > +int > +f (int n) > +{ > + switch (n) > + { > + case 1: > + [[fallthrough, fallthrough]]; // { dg-warning "attribute specified multiple times" } > + case 2: > + [[fallthrough]] [[fallthrough]]; // { dg-warning "attribute specified multiple times" } > + case 3: > + return 15; > + } > + > + if (n == 10) > + [[likely]] [[likely]] return 42; // { dg-warning "ignoring attribute" } > + else if (n == 11) > + [[unlikely]] [[unlikely]] return 10; // { dg-warning "ignoring attribute" } > + else if (n == 12) > + [[likely, likely]] return 42; // { dg-warning "ignoring attribute" } > + else > + [[unlikely, unlikely]] return 0; // { dg-warning "ignoring attribute" } > +} > diff --git a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C > index 12c75c7ba73..1669bbf5376 100644 > --- a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C > +++ b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C > @@ -1,4 +1,4 @@ > // PR c++/60365 > // { dg-do compile { target c++14 } } > > -void func [[deprecated, deprecated]] (); // { dg-error "at most once" } > +void func [[deprecated, deprecated]] (); > diff --git a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C > index c810fd0daad..da992f9f0c7 100644 > --- a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C > +++ b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C > @@ -2,7 +2,7 @@ > /* { dg-do compile { target c++20 } } */ > /* { dg-options "-O -ftrack-macro-expansion=0" } */ > > -[[nodiscard, nodiscard]] int check1 (void); /* { dg-error "nodiscard\[^\n\r]*can appear at most once" } */ > +[[nodiscard, nodiscard]] int check1 (void); > > void > test (void) > > base-commit: 05f1883cfd041adba5f595e97118100b6e00a524 >
On Fri, Nov 06, 2020 at 02:23:10PM -0500, Jason Merrill via Gcc-patches wrote: > On 11/6/20 2:06 PM, Marek Polacek wrote: > > Following Joseph's change for C to allow duplicate C2x standard attributes > > <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, > > this patch does a similar thing for C++. This is DR 1914, to be resolved by > > <wg21.link/p2156>, which is not part of the standard yet, but has a wide > > support so look like a shoo-in. Some duplications still produce warnings; > > I didn't change that because a warning might be desirable. > > What's the rationale for warning about some and not others? I don't have any. Joseph's patch removed the error for a duplicated 'fallthrough' attribute, but the warning remained so I left it as-is too. So either we just downgrade the error to a warning, or remove the remaining warnings too. I think I slightly prefer the former; with perhaps a small tweak not to warn when the duplicated attribute comes from a macro expansion. Marek
On 11/6/20 2:34 PM, Marek Polacek wrote: > On Fri, Nov 06, 2020 at 02:23:10PM -0500, Jason Merrill via Gcc-patches wrote: >> On 11/6/20 2:06 PM, Marek Polacek wrote: >>> Following Joseph's change for C to allow duplicate C2x standard attributes >>> <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, >>> this patch does a similar thing for C++. This is DR 1914, to be resolved by >>> <wg21.link/p2156>, which is not part of the standard yet, but has a wide >>> support so look like a shoo-in. Some duplications still produce warnings; >>> I didn't change that because a warning might be desirable. >> >> What's the rationale for warning about some and not others? > > I don't have any. Joseph's patch removed the error for a duplicated > 'fallthrough' attribute, but the warning remained so I left it as-is > too. > > So either we just downgrade the error to a warning, or remove the > remaining warnings too. I think I slightly prefer the former; with perhaps > a small tweak not to warn when the duplicated attribute comes from a macro > expansion. Sounds good. Jason
On Fri, Nov 06, 2020 at 03:01:56PM -0500, Jason Merrill via Gcc-patches wrote: > On 11/6/20 2:34 PM, Marek Polacek wrote: > > On Fri, Nov 06, 2020 at 02:23:10PM -0500, Jason Merrill via Gcc-patches wrote: > > > On 11/6/20 2:06 PM, Marek Polacek wrote: > > > > Following Joseph's change for C to allow duplicate C2x standard attributes > > > > <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, > > > > this patch does a similar thing for C++. This is DR 1914, to be resolved by > > > > <wg21.link/p2156>, which is not part of the standard yet, but has a wide > > > > support so look like a shoo-in. Some duplications still produce warnings; > > > > I didn't change that because a warning might be desirable. > > > > > > What's the rationale for warning about some and not others? > > > > I don't have any. Joseph's patch removed the error for a duplicated > > 'fallthrough' attribute, but the warning remained so I left it as-is > > too. > > > > So either we just downgrade the error to a warning, or remove the > > remaining warnings too. I think I slightly prefer the former; with perhaps > > a small tweak not to warn when the duplicated attribute comes from a macro > > expansion. > > Sounds good. Here's a patch that does that. cp_parser_check_std_attribute now handles more standard attributes. There's still a discrepancy that we warn about [[fallthrough]] [[fallthrough]] but not about [[noreturn]] [[noreturn]]. Fixing this would involve calling cp_parser_check_std_attribute in another spot (in a loop), presumably. But I suspect that's less important for now. Thanks, Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Following Joseph's change for C to allow duplicate C2x standard attributes <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, this patch does a similar thing for C++. This is DR 1914, to be resolved by <wg21.link/p2156>, which is not part of the standard yet, but has wide support so looks like a shoo-in. The duplications now produce warnings instead, but only if the attribute wasn't specified via a macro. gcc/c-family/ChangeLog: DR 1914 * c-common.c (attribute_fallthrough_p): Tweak the warning message. gcc/cp/ChangeLog: DR 1914 * parser.c (cp_parser_check_std_attribute): Return bool. Add a location_t parameter. Return true if the attribute wasn't duplicated. Give a warning instead of an error. Check more attributes. (cp_parser_std_attribute_list): Don't add duplicated attributes to the list. Pass location to cp_parser_check_std_attribute. gcc/testsuite/ChangeLog: DR 1914 * c-c++-common/attr-fallthrough-2.c: Adjust dg-warning. * g++.dg/cpp0x/fallthrough2.C: Likewise. * g++.dg/cpp0x/gen-attrs-60.C: Turn dg-error into dg-warning. * g++.dg/cpp1y/attr-deprecated-2.C: Likewise. * g++.dg/cpp2a/attr-likely2.C: Adjust dg-warning. * g++.dg/cpp2a/nodiscard-once.C: Turn dg-error into dg-warning. * g++.dg/cpp0x/gen-attrs-72.C: New test. --- gcc/c-family/c-common.c | 5 +- gcc/cp/parser.c | 51 ++++++++++--------- .../c-c++-common/attr-fallthrough-2.c | 2 +- gcc/testsuite/g++.dg/cpp0x/fallthrough2.C | 2 +- gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C | 2 +- gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C | 45 ++++++++++++++++ .../g++.dg/cpp1y/attr-deprecated-2.C | 2 +- gcc/testsuite/g++.dg/cpp2a/attr-likely2.C | 2 +- gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C | 2 +- 9 files changed, 81 insertions(+), 32 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index d4d3228b8f6..29508bca97b 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -5752,9 +5752,10 @@ attribute_fallthrough_p (tree attr) tree t = lookup_attribute ("fallthrough", attr); if (t == NULL_TREE) return false; - /* This attribute shall appear at most once in each attribute-list. */ + /* It is no longer true that "this attribute shall appear at most once in + each attribute-list", but we still give a warning. */ if (lookup_attribute ("fallthrough", TREE_CHAIN (t))) - warning (OPT_Wattributes, "%<fallthrough%> attribute specified multiple " + warning (OPT_Wattributes, "attribute %<fallthrough%> specified multiple " "times"); /* No attribute-argument-clause shall be present. */ else if (TREE_VALUE (t) != NULL_TREE) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 323d7424a83..6fcee3efe6f 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -27269,30 +27269,30 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) return attribute; } -/* Check that the attribute ATTRIBUTE appears at most once in the - attribute-list ATTRIBUTES. This is enforced for noreturn (7.6.3), - nodiscard, and deprecated (7.6.5). Note that - carries_dependency (7.6.4) isn't implemented yet in GCC. */ +/* Warn if the attribute ATTRIBUTE appears more than once in the + attribute-list ATTRIBUTES. This used to be enforced for certain + attributes, but the restriction was removed in P2156. Note that + carries_dependency ([dcl.attr.depend]) isn't implemented yet in GCC. + LOC is the location of ATTRIBUTE. Returns true if ATTRIBUTE was not + found in ATTRIBUTES. */ -static void -cp_parser_check_std_attribute (tree attributes, tree attribute) +static bool +cp_parser_check_std_attribute (location_t loc, tree attributes, tree attribute) { + static auto alist = { "noreturn", "deprecated", "nodiscard", "maybe_unused", + "likely", "unlikely", "fallthrough", + "no_unique_address" }; if (attributes) - { - tree name = get_attribute_name (attribute); - if (is_attribute_p ("noreturn", name) - && lookup_attribute ("noreturn", attributes)) - error ("attribute %<noreturn%> can appear at most once " - "in an attribute-list"); - else if (is_attribute_p ("deprecated", name) - && lookup_attribute ("deprecated", attributes)) - error ("attribute %<deprecated%> can appear at most once " - "in an attribute-list"); - else if (is_attribute_p ("nodiscard", name) - && lookup_attribute ("nodiscard", attributes)) - error ("attribute %<nodiscard%> can appear at most once " - "in an attribute-list"); - } + for (const auto &a : alist) + if (is_attribute_p (a, get_attribute_name (attribute)) + && lookup_attribute (a, attributes)) + { + if (!from_macro_expansion_at (loc)) + warning_at (loc, OPT_Wattributes, "attribute %qs specified " + "multiple times", a); + return false; + } + return true; } /* Parse a list of standard C++-11 attributes. @@ -27312,14 +27312,17 @@ cp_parser_std_attribute_list (cp_parser *parser, tree attr_ns) while (true) { + location_t loc = cp_lexer_peek_token (parser->lexer)->location; attribute = cp_parser_std_attribute (parser, attr_ns); if (attribute == error_mark_node) break; if (attribute != NULL_TREE) { - cp_parser_check_std_attribute (attributes, attribute); - TREE_CHAIN (attribute) = attributes; - attributes = attribute; + if (cp_parser_check_std_attribute (loc, attributes, attribute)) + { + TREE_CHAIN (attribute) = attributes; + attributes = attribute; + } } token = cp_lexer_peek_token (parser->lexer); if (token->type == CPP_ELLIPSIS) diff --git a/gcc/testsuite/c-c++-common/attr-fallthrough-2.c b/gcc/testsuite/c-c++-common/attr-fallthrough-2.c index be61d5e6666..156b413db7a 100644 --- a/gcc/testsuite/c-c++-common/attr-fallthrough-2.c +++ b/gcc/testsuite/c-c++-common/attr-fallthrough-2.c @@ -34,7 +34,7 @@ fn (int i) __attribute__((fallthrough ("x"))); /* { dg-warning "specified with a parameter" } */ case 7: bar (1); - __attribute__((fallthrough, fallthrough)); /* { dg-warning "attribute specified multiple times" } */ + __attribute__((fallthrough, fallthrough)); /* { dg-warning "specified multiple times" } */ case 8: bar (1); __attribute__((fallthrough)); diff --git a/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C b/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C index f2d0ce1c693..071c2cb09eb 100644 --- a/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C +++ b/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C @@ -14,7 +14,7 @@ f (int i) [[fallthrough]]; case 3: bar (1); - [[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. attribute specified multiple times" } + [[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. specified multiple times" } case 2: bar (2); } diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C index cb0c31ec63f..9203d1dd1bd 100644 --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C @@ -1,4 +1,4 @@ // PR c++/60365 // { dg-do compile { target c++11 } } -void func [[noreturn, noreturn]] (); // { dg-error "at most once" } +void func [[noreturn, noreturn]] (); // { dg-warning "specified multiple times" } diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C new file mode 100644 index 00000000000..3c235b5e921 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C @@ -0,0 +1,45 @@ +// DR 1914 - Duplicate standard attributes +// { dg-do compile { target c++11 } } + +#define ATTR_NORETURN [[noreturn, noreturn]] + +[[noreturn, noreturn]] void fn0(); // { dg-warning "specified multiple times" } +ATTR_NORETURN void fn0a(); +[[noreturn]] [[noreturn]] void fn1(); +[[deprecated, deprecated]] void fn2(); // { dg-warning "specified multiple times" } +[[deprecated]] [[deprecated]] void fn3(); +[[maybe_unused]] [[maybe_unused]] int fn4(); +[[maybe_unused, maybe_unused]] int fn5(); // { dg-warning "specified multiple times" } +[[nodiscard]] [[nodiscard]] int fn6(); +[[nodiscard, nodiscard]] int fn7(); // { dg-warning "specified multiple times" } + +struct E { }; +struct A { + [[no_unique_address]] [[no_unique_address]] E e; +}; +struct B { + [[no_unique_address, no_unique_address]] E e; // { dg-warning "specified multiple times" } +}; + +int +f (int n) +{ + switch (n) + { + case 1: + [[fallthrough, fallthrough]]; // { dg-warning "specified multiple times" } + case 2: + [[fallthrough]] [[fallthrough]]; // { dg-warning "specified multiple times" } + case 3: + return 15; + } + + if (n == 10) + [[likely]] [[likely]] return 42; // { dg-warning "ignoring attribute" } + else if (n == 11) + [[unlikely]] [[unlikely]] return 10; // { dg-warning "ignoring attribute" } + else if (n == 12) + [[likely, likely]] return 42; // { dg-warning "specified multiple times" } + else + [[unlikely, unlikely]] return 0; // { dg-warning "specified multiple times" } +} diff --git a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C index 12c75c7ba73..ac6c4aecf9b 100644 --- a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C +++ b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C @@ -1,4 +1,4 @@ // PR c++/60365 // { dg-do compile { target c++14 } } -void func [[deprecated, deprecated]] (); // { dg-error "at most once" } +void func [[deprecated, deprecated]] (); // { dg-warning "specified multiple times" } diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C index ee178dec9c5..0bc5f1e1ae7 100644 --- a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C +++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C @@ -4,7 +4,7 @@ bool b; int main() { if (b) - [[likely, likely]] b; // { dg-warning "ignoring" } + [[likely, likely]] b; // { dg-warning "specified multiple times" } else [[unlikely]] [[likely]] b; // { dg-warning "ignoring" } diff --git a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C index c810fd0daad..c95fa1b0741 100644 --- a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C +++ b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C @@ -2,7 +2,7 @@ /* { dg-do compile { target c++20 } } */ /* { dg-options "-O -ftrack-macro-expansion=0" } */ -[[nodiscard, nodiscard]] int check1 (void); /* { dg-error "nodiscard\[^\n\r]*can appear at most once" } */ +[[nodiscard, nodiscard]] int check1 (void); // { dg-warning "specified multiple times" } void test (void) base-commit: b642fca1c31b2e2175e0860daf32b4ee0d918085
On 11/9/20 12:05 PM, Marek Polacek wrote: > On Fri, Nov 06, 2020 at 03:01:56PM -0500, Jason Merrill via Gcc-patches wrote: >> On 11/6/20 2:34 PM, Marek Polacek wrote: >>> On Fri, Nov 06, 2020 at 02:23:10PM -0500, Jason Merrill via Gcc-patches wrote: >>>> On 11/6/20 2:06 PM, Marek Polacek wrote: >>>>> Following Joseph's change for C to allow duplicate C2x standard attributes >>>>> <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, >>>>> this patch does a similar thing for C++. This is DR 1914, to be resolved by >>>>> <wg21.link/p2156>, which is not part of the standard yet, but has a wide >>>>> support so look like a shoo-in. Some duplications still produce warnings; >>>>> I didn't change that because a warning might be desirable. >>>> >>>> What's the rationale for warning about some and not others? >>> >>> I don't have any. Joseph's patch removed the error for a duplicated >>> 'fallthrough' attribute, but the warning remained so I left it as-is >>> too. >>> >>> So either we just downgrade the error to a warning, or remove the >>> remaining warnings too. I think I slightly prefer the former; with perhaps >>> a small tweak not to warn when the duplicated attribute comes from a macro >>> expansion. >> >> Sounds good. > > Here's a patch that does that. cp_parser_check_std_attribute now handles > more standard attributes. There's still a discrepancy that we warn about > [[fallthrough]] [[fallthrough]] but not about [[noreturn]] [[noreturn]]. > Fixing this would involve calling cp_parser_check_std_attribute in another > spot (in a loop), presumably. But I suspect that's less important for now. > > Thanks, > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > Following Joseph's change for C to allow duplicate C2x standard attributes > <https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557272.html>, > this patch does a similar thing for C++. This is DR 1914, to be resolved by > <wg21.link/p2156>, which is not part of the standard yet, but has wide > support so looks like a shoo-in. The duplications now produce warnings > instead, but only if the attribute wasn't specified via a macro. > > gcc/c-family/ChangeLog: > > DR 1914 > * c-common.c (attribute_fallthrough_p): Tweak the warning > message. > > gcc/cp/ChangeLog: > > DR 1914 > * parser.c (cp_parser_check_std_attribute): Return bool. Add a > location_t parameter. Return true if the attribute wasn't duplicated. > Give a warning instead of an error. Check more attributes. > (cp_parser_std_attribute_list): Don't add duplicated attributes to > the list. Pass location to cp_parser_check_std_attribute. > > gcc/testsuite/ChangeLog: > > DR 1914 > * c-c++-common/attr-fallthrough-2.c: Adjust dg-warning. > * g++.dg/cpp0x/fallthrough2.C: Likewise. > * g++.dg/cpp0x/gen-attrs-60.C: Turn dg-error into dg-warning. > * g++.dg/cpp1y/attr-deprecated-2.C: Likewise. > * g++.dg/cpp2a/attr-likely2.C: Adjust dg-warning. > * g++.dg/cpp2a/nodiscard-once.C: Turn dg-error into dg-warning. > * g++.dg/cpp0x/gen-attrs-72.C: New test. > --- > gcc/c-family/c-common.c | 5 +- > gcc/cp/parser.c | 51 ++++++++++--------- > .../c-c++-common/attr-fallthrough-2.c | 2 +- > gcc/testsuite/g++.dg/cpp0x/fallthrough2.C | 2 +- > gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C | 2 +- > gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C | 45 ++++++++++++++++ > .../g++.dg/cpp1y/attr-deprecated-2.C | 2 +- > gcc/testsuite/g++.dg/cpp2a/attr-likely2.C | 2 +- > gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C | 2 +- > 9 files changed, 81 insertions(+), 32 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index d4d3228b8f6..29508bca97b 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -5752,9 +5752,10 @@ attribute_fallthrough_p (tree attr) > tree t = lookup_attribute ("fallthrough", attr); > if (t == NULL_TREE) > return false; > - /* This attribute shall appear at most once in each attribute-list. */ > + /* It is no longer true that "this attribute shall appear at most once in > + each attribute-list", but we still give a warning. */ > if (lookup_attribute ("fallthrough", TREE_CHAIN (t))) > - warning (OPT_Wattributes, "%<fallthrough%> attribute specified multiple " > + warning (OPT_Wattributes, "attribute %<fallthrough%> specified multiple " > "times"); > /* No attribute-argument-clause shall be present. */ > else if (TREE_VALUE (t) != NULL_TREE) > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index 323d7424a83..6fcee3efe6f 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -27269,30 +27269,30 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) > return attribute; > } > > -/* Check that the attribute ATTRIBUTE appears at most once in the > - attribute-list ATTRIBUTES. This is enforced for noreturn (7.6.3), > - nodiscard, and deprecated (7.6.5). Note that > - carries_dependency (7.6.4) isn't implemented yet in GCC. */ > +/* Warn if the attribute ATTRIBUTE appears more than once in the > + attribute-list ATTRIBUTES. This used to be enforced for certain > + attributes, but the restriction was removed in P2156. Note that > + carries_dependency ([dcl.attr.depend]) isn't implemented yet in GCC. > + LOC is the location of ATTRIBUTE. Returns true if ATTRIBUTE was not > + found in ATTRIBUTES. */ > > -static void > -cp_parser_check_std_attribute (tree attributes, tree attribute) > +static bool > +cp_parser_check_std_attribute (location_t loc, tree attributes, tree attribute) > { > + static auto alist = { "noreturn", "deprecated", "nodiscard", "maybe_unused", > + "likely", "unlikely", "fallthrough", > + "no_unique_address" }; > if (attributes) > - { > - tree name = get_attribute_name (attribute); > - if (is_attribute_p ("noreturn", name) > - && lookup_attribute ("noreturn", attributes)) > - error ("attribute %<noreturn%> can appear at most once " > - "in an attribute-list"); > - else if (is_attribute_p ("deprecated", name) > - && lookup_attribute ("deprecated", attributes)) > - error ("attribute %<deprecated%> can appear at most once " > - "in an attribute-list"); > - else if (is_attribute_p ("nodiscard", name) > - && lookup_attribute ("nodiscard", attributes)) > - error ("attribute %<nodiscard%> can appear at most once " > - "in an attribute-list"); > - } > + for (const auto &a : alist) > + if (is_attribute_p (a, get_attribute_name (attribute)) > + && lookup_attribute (a, attributes)) > + { > + if (!from_macro_expansion_at (loc)) > + warning_at (loc, OPT_Wattributes, "attribute %qs specified " > + "multiple times", a); > + return false; > + } > + return true; > } > > /* Parse a list of standard C++-11 attributes. > @@ -27312,14 +27312,17 @@ cp_parser_std_attribute_list (cp_parser *parser, tree attr_ns) > > while (true) > { > + location_t loc = cp_lexer_peek_token (parser->lexer)->location; > attribute = cp_parser_std_attribute (parser, attr_ns); > if (attribute == error_mark_node) > break; > if (attribute != NULL_TREE) > { > - cp_parser_check_std_attribute (attributes, attribute); > - TREE_CHAIN (attribute) = attributes; > - attributes = attribute; > + if (cp_parser_check_std_attribute (loc, attributes, attribute)) You might combine this condition with the enclosing one. OK either way. > + { > + TREE_CHAIN (attribute) = attributes; > + attributes = attribute; > + } > } > token = cp_lexer_peek_token (parser->lexer); > if (token->type == CPP_ELLIPSIS) > diff --git a/gcc/testsuite/c-c++-common/attr-fallthrough-2.c b/gcc/testsuite/c-c++-common/attr-fallthrough-2.c > index be61d5e6666..156b413db7a 100644 > --- a/gcc/testsuite/c-c++-common/attr-fallthrough-2.c > +++ b/gcc/testsuite/c-c++-common/attr-fallthrough-2.c > @@ -34,7 +34,7 @@ fn (int i) > __attribute__((fallthrough ("x"))); /* { dg-warning "specified with a parameter" } */ > case 7: > bar (1); > - __attribute__((fallthrough, fallthrough)); /* { dg-warning "attribute specified multiple times" } */ > + __attribute__((fallthrough, fallthrough)); /* { dg-warning "specified multiple times" } */ > case 8: > bar (1); > __attribute__((fallthrough)); > diff --git a/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C b/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C > index f2d0ce1c693..071c2cb09eb 100644 > --- a/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C > +++ b/gcc/testsuite/g++.dg/cpp0x/fallthrough2.C > @@ -14,7 +14,7 @@ f (int i) > [[fallthrough]]; > case 3: > bar (1); > - [[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. attribute specified multiple times" } > + [[gnu::fallthrough, gnu::fallthrough]]; // { dg-warning ".fallthrough. specified multiple times" } > case 2: > bar (2); > } > diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C > index cb0c31ec63f..9203d1dd1bd 100644 > --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C > +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C > @@ -1,4 +1,4 @@ > // PR c++/60365 > // { dg-do compile { target c++11 } } > > -void func [[noreturn, noreturn]] (); // { dg-error "at most once" } > +void func [[noreturn, noreturn]] (); // { dg-warning "specified multiple times" } > diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C > new file mode 100644 > index 00000000000..3c235b5e921 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C > @@ -0,0 +1,45 @@ > +// DR 1914 - Duplicate standard attributes > +// { dg-do compile { target c++11 } } > + > +#define ATTR_NORETURN [[noreturn, noreturn]] > + > +[[noreturn, noreturn]] void fn0(); // { dg-warning "specified multiple times" } > +ATTR_NORETURN void fn0a(); > +[[noreturn]] [[noreturn]] void fn1(); > +[[deprecated, deprecated]] void fn2(); // { dg-warning "specified multiple times" } > +[[deprecated]] [[deprecated]] void fn3(); > +[[maybe_unused]] [[maybe_unused]] int fn4(); > +[[maybe_unused, maybe_unused]] int fn5(); // { dg-warning "specified multiple times" } > +[[nodiscard]] [[nodiscard]] int fn6(); > +[[nodiscard, nodiscard]] int fn7(); // { dg-warning "specified multiple times" } > + > +struct E { }; > +struct A { > + [[no_unique_address]] [[no_unique_address]] E e; > +}; > +struct B { > + [[no_unique_address, no_unique_address]] E e; // { dg-warning "specified multiple times" } > +}; > + > +int > +f (int n) > +{ > + switch (n) > + { > + case 1: > + [[fallthrough, fallthrough]]; // { dg-warning "specified multiple times" } > + case 2: > + [[fallthrough]] [[fallthrough]]; // { dg-warning "specified multiple times" } > + case 3: > + return 15; > + } > + > + if (n == 10) > + [[likely]] [[likely]] return 42; // { dg-warning "ignoring attribute" } > + else if (n == 11) > + [[unlikely]] [[unlikely]] return 10; // { dg-warning "ignoring attribute" } > + else if (n == 12) > + [[likely, likely]] return 42; // { dg-warning "specified multiple times" } > + else > + [[unlikely, unlikely]] return 0; // { dg-warning "specified multiple times" } > +} > diff --git a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C > index 12c75c7ba73..ac6c4aecf9b 100644 > --- a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C > +++ b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C > @@ -1,4 +1,4 @@ > // PR c++/60365 > // { dg-do compile { target c++14 } } > > -void func [[deprecated, deprecated]] (); // { dg-error "at most once" } > +void func [[deprecated, deprecated]] (); // { dg-warning "specified multiple times" } > diff --git a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C > index ee178dec9c5..0bc5f1e1ae7 100644 > --- a/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C > +++ b/gcc/testsuite/g++.dg/cpp2a/attr-likely2.C > @@ -4,7 +4,7 @@ bool b; > int main() > { > if (b) > - [[likely, likely]] b; // { dg-warning "ignoring" } > + [[likely, likely]] b; // { dg-warning "specified multiple times" } > else > [[unlikely]] [[likely]] b; // { dg-warning "ignoring" } > > diff --git a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C > index c810fd0daad..c95fa1b0741 100644 > --- a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C > +++ b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C > @@ -2,7 +2,7 @@ > /* { dg-do compile { target c++20 } } */ > /* { dg-options "-O -ftrack-macro-expansion=0" } */ > > -[[nodiscard, nodiscard]] int check1 (void); /* { dg-error "nodiscard\[^\n\r]*can appear at most once" } */ > +[[nodiscard, nodiscard]] int check1 (void); // { dg-warning "specified multiple times" } > > void > test (void) > > base-commit: b642fca1c31b2e2175e0860daf32b4ee0d918085 >
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 232a4797c09..03d8fec9936 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -5751,7 +5751,8 @@ attribute_fallthrough_p (tree attr) tree t = lookup_attribute ("fallthrough", attr); if (t == NULL_TREE) return false; - /* This attribute shall appear at most once in each attribute-list. */ + /* It is no longer true that "this attribute shall appear at most once in + each attribute-list", but we still give a warning. */ if (lookup_attribute ("fallthrough", TREE_CHAIN (t))) warning (OPT_Wattributes, "%<fallthrough%> attribute specified multiple " "times"); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 6e7b982f073..0567646fbe2 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -27269,32 +27269,6 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) return attribute; } -/* Check that the attribute ATTRIBUTE appears at most once in the - attribute-list ATTRIBUTES. This is enforced for noreturn (7.6.3), - nodiscard, and deprecated (7.6.5). Note that - carries_dependency (7.6.4) isn't implemented yet in GCC. */ - -static void -cp_parser_check_std_attribute (tree attributes, tree attribute) -{ - if (attributes) - { - tree name = get_attribute_name (attribute); - if (is_attribute_p ("noreturn", name) - && lookup_attribute ("noreturn", attributes)) - error ("attribute %<noreturn%> can appear at most once " - "in an attribute-list"); - else if (is_attribute_p ("deprecated", name) - && lookup_attribute ("deprecated", attributes)) - error ("attribute %<deprecated%> can appear at most once " - "in an attribute-list"); - else if (is_attribute_p ("nodiscard", name) - && lookup_attribute ("nodiscard", attributes)) - error ("attribute %<nodiscard%> can appear at most once " - "in an attribute-list"); - } -} - /* Parse a list of standard C++-11 attributes. attribute-list: @@ -27317,7 +27291,6 @@ cp_parser_std_attribute_list (cp_parser *parser, tree attr_ns) break; if (attribute != NULL_TREE) { - cp_parser_check_std_attribute (attributes, attribute); TREE_CHAIN (attribute) = attributes; attributes = attribute; } diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C index cb0c31ec63f..4e905c394ca 100644 --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-60.C @@ -1,4 +1,4 @@ // PR c++/60365 // { dg-do compile { target c++11 } } -void func [[noreturn, noreturn]] (); // { dg-error "at most once" } +void func [[noreturn, noreturn]] (); diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C new file mode 100644 index 00000000000..0cb874b6191 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-72.C @@ -0,0 +1,42 @@ +// DR 1914 - Duplicate standard attributes +// { dg-do compile { target c++11 } } + +[[noreturn, noreturn]] void fn0(); +[[noreturn]] [[noreturn]] void fn1(); +[[deprecated, deprecated]] void fn2(); +[[deprecated]] [[deprecated]] void fn3(); +[[maybe_unused]] [[maybe_unused]] int fn4(); +[[maybe_unused, maybe_unused]] int fn5(); +[[nodiscard]] [[nodiscard]] int fn6(); +[[nodiscard, nodiscard]] int fn7(); + +struct E { }; +struct A { + [[no_unique_address]] [[no_unique_address]] E e; +}; +struct B { + [[no_unique_address, no_unique_address]] E e; +}; + +int +f (int n) +{ + switch (n) + { + case 1: + [[fallthrough, fallthrough]]; // { dg-warning "attribute specified multiple times" } + case 2: + [[fallthrough]] [[fallthrough]]; // { dg-warning "attribute specified multiple times" } + case 3: + return 15; + } + + if (n == 10) + [[likely]] [[likely]] return 42; // { dg-warning "ignoring attribute" } + else if (n == 11) + [[unlikely]] [[unlikely]] return 10; // { dg-warning "ignoring attribute" } + else if (n == 12) + [[likely, likely]] return 42; // { dg-warning "ignoring attribute" } + else + [[unlikely, unlikely]] return 0; // { dg-warning "ignoring attribute" } +} diff --git a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C index 12c75c7ba73..1669bbf5376 100644 --- a/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C +++ b/gcc/testsuite/g++.dg/cpp1y/attr-deprecated-2.C @@ -1,4 +1,4 @@ // PR c++/60365 // { dg-do compile { target c++14 } } -void func [[deprecated, deprecated]] (); // { dg-error "at most once" } +void func [[deprecated, deprecated]] (); diff --git a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C index c810fd0daad..da992f9f0c7 100644 --- a/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C +++ b/gcc/testsuite/g++.dg/cpp2a/nodiscard-once.C @@ -2,7 +2,7 @@ /* { dg-do compile { target c++20 } } */ /* { dg-options "-O -ftrack-macro-expansion=0" } */ -[[nodiscard, nodiscard]] int check1 (void); /* { dg-error "nodiscard\[^\n\r]*can appear at most once" } */ +[[nodiscard, nodiscard]] int check1 (void); void test (void)