| Message ID | 20190616191001.GM5989@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | C++ PATCH for c++/83820 - excessive attribute arguments not detected | expand |
On 6/16/19 1:10 PM, Marek Polacek wrote: > While messing with [[noreturn]] I also found out that we don't detect > the case when an attribute specifier that takes no arguments contains > an attribute-argument-clause. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2019-06-16 Marek Polacek <polacek@redhat.com> > > PR c++/83820 - excessive attribute arguments not detected. > * parser.c (cp_parser_std_attribute): Detect excessive arguments. > > * g++.dg/cpp0x/gen-attrs-67.C: New test. > > diff --git gcc/cp/parser.c gcc/cp/parser.c > index 8f5ae84670a..871bc45da63 100644 > --- gcc/cp/parser.c > +++ gcc/cp/parser.c > @@ -26149,6 +26149,20 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) > vec<tree, va_gc> *vec; > int attr_flag = normal_attr; > > + /* Maybe we don't expect to see any arguments for this attribute. */ > + const attribute_spec *as > + = lookup_attribute_spec (TREE_PURPOSE (attribute)); > + if (as && as->max_length == 0) > + { > + error_at (token->location, "attribute %qE does not take any arguments", > + attr_id); Not to be too anal about this but most messages have the word order reversed and would be phrased as %qE attribute does not take any arguments I've been adjusting the order to match this form as I notice it so it would be great if we could use this form in new diagnostics as well. Thanks! Martin > + cp_parser_skip_to_closing_parenthesis (parser, > + /*recovering=*/true, > + /*or_comma=*/false, > + /*consume_paren=*/true); > + return error_mark_node; > + } > + > if (attr_ns == gnu_identifier > && attribute_takes_identifier_p (attr_id)) > /* A GNU attribute that takes an identifier in parameter. */ > diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C > new file mode 100644 > index 00000000000..bbbedd0240a > --- /dev/null > +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C > @@ -0,0 +1,11 @@ > +// PR c++/83820 - excessive attribute arguments not detected. > +// { dg-do compile { target c++11 } } > + > +[[noreturn()]] void f0 (); // { dg-error "attribute .noreturn. does not take any arguments" } > +[[noreturn(1)]] void f1 (); // { dg-error "attribute .noreturn. does not take any arguments" } > +[[noreturn(1, 2)]] void f2 (); // { dg-error "attribute .noreturn. does not take any arguments" } > +[[maybe_unused()]] int f3(); // { dg-error "attribute .maybe_unused. does not take any arguments" } > +[[nodiscard()]] int f4(); // { dg-error "attribute .nodiscard. does not take any arguments" } > +[[gnu::noinline()]] int f5(); // { dg-error "attribute .noinline. does not take any arguments" } > +[[gnu::constructor]] int f6(); > +[[gnu::constructor(101)]] int f7(); >
Ok. On Sun, Jun 16, 2019, 3:10 PM Marek Polacek <polacek@redhat.com> wrote: > While messing with [[noreturn]] I also found out that we don't detect > the case when an attribute specifier that takes no arguments contains > an attribute-argument-clause. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2019-06-16 Marek Polacek <polacek@redhat.com> > > PR c++/83820 - excessive attribute arguments not detected. > * parser.c (cp_parser_std_attribute): Detect excessive arguments. > > * g++.dg/cpp0x/gen-attrs-67.C: New test. > > diff --git gcc/cp/parser.c gcc/cp/parser.c > index 8f5ae84670a..871bc45da63 100644 > --- gcc/cp/parser.c > +++ gcc/cp/parser.c > @@ -26149,6 +26149,20 @@ cp_parser_std_attribute (cp_parser *parser, tree > attr_ns) > vec<tree, va_gc> *vec; > int attr_flag = normal_attr; > > + /* Maybe we don't expect to see any arguments for this attribute. */ > + const attribute_spec *as > + = lookup_attribute_spec (TREE_PURPOSE (attribute)); > + if (as && as->max_length == 0) > + { > + error_at (token->location, "attribute %qE does not take any > arguments", > + attr_id); > + cp_parser_skip_to_closing_parenthesis (parser, > + /*recovering=*/true, > + /*or_comma=*/false, > + /*consume_paren=*/true); > + return error_mark_node; > + } > + > if (attr_ns == gnu_identifier > && attribute_takes_identifier_p (attr_id)) > /* A GNU attribute that takes an identifier in parameter. */ > diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C > gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C > new file mode 100644 > index 00000000000..bbbedd0240a > --- /dev/null > +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C > @@ -0,0 +1,11 @@ > +// PR c++/83820 - excessive attribute arguments not detected. > +// { dg-do compile { target c++11 } } > + > +[[noreturn()]] void f0 (); // { dg-error "attribute .noreturn. does not > take any arguments" } > +[[noreturn(1)]] void f1 (); // { dg-error "attribute .noreturn. does not > take any arguments" } > +[[noreturn(1, 2)]] void f2 (); // { dg-error "attribute .noreturn. does > not take any arguments" } > +[[maybe_unused()]] int f3(); // { dg-error "attribute .maybe_unused. does > not take any arguments" } > +[[nodiscard()]] int f4(); // { dg-error "attribute .nodiscard. does not > take any arguments" } > +[[gnu::noinline()]] int f5(); // { dg-error "attribute .noinline. does > not take any arguments" } > +[[gnu::constructor]] int f6(); > +[[gnu::constructor(101)]] int f7(); >
On Mon, Jun 17, 2019 at 09:50:51AM -0600, Martin Sebor wrote: > On 6/16/19 1:10 PM, Marek Polacek wrote: > > While messing with [[noreturn]] I also found out that we don't detect > > the case when an attribute specifier that takes no arguments contains > > an attribute-argument-clause. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2019-06-16 Marek Polacek <polacek@redhat.com> > > > > PR c++/83820 - excessive attribute arguments not detected. > > * parser.c (cp_parser_std_attribute): Detect excessive arguments. > > > > * g++.dg/cpp0x/gen-attrs-67.C: New test. > > > > diff --git gcc/cp/parser.c gcc/cp/parser.c > > index 8f5ae84670a..871bc45da63 100644 > > --- gcc/cp/parser.c > > +++ gcc/cp/parser.c > > @@ -26149,6 +26149,20 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) > > vec<tree, va_gc> *vec; > > int attr_flag = normal_attr; > > + /* Maybe we don't expect to see any arguments for this attribute. */ > > + const attribute_spec *as > > + = lookup_attribute_spec (TREE_PURPOSE (attribute)); > > + if (as && as->max_length == 0) > > + { > > + error_at (token->location, "attribute %qE does not take any arguments", > > + attr_id); > > Not to be too anal about this but most messages have the word order > reversed and would be phrased as > > %qE attribute does not take any arguments > > I've been adjusting the order to match this form as I notice it so > it would be great if we could use this form in new diagnostics as > well. *shrug* I have no problem changing that. Will commit with your proposed change. Thanks, -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
diff --git gcc/cp/parser.c gcc/cp/parser.c index 8f5ae84670a..871bc45da63 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -26149,6 +26149,20 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) vec<tree, va_gc> *vec; int attr_flag = normal_attr; + /* Maybe we don't expect to see any arguments for this attribute. */ + const attribute_spec *as + = lookup_attribute_spec (TREE_PURPOSE (attribute)); + if (as && as->max_length == 0) + { + error_at (token->location, "attribute %qE does not take any arguments", + attr_id); + cp_parser_skip_to_closing_parenthesis (parser, + /*recovering=*/true, + /*or_comma=*/false, + /*consume_paren=*/true); + return error_mark_node; + } + if (attr_ns == gnu_identifier && attribute_takes_identifier_p (attr_id)) /* A GNU attribute that takes an identifier in parameter. */ diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C new file mode 100644 index 00000000000..bbbedd0240a --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-67.C @@ -0,0 +1,11 @@ +// PR c++/83820 - excessive attribute arguments not detected. +// { dg-do compile { target c++11 } } + +[[noreturn()]] void f0 (); // { dg-error "attribute .noreturn. does not take any arguments" } +[[noreturn(1)]] void f1 (); // { dg-error "attribute .noreturn. does not take any arguments" } +[[noreturn(1, 2)]] void f2 (); // { dg-error "attribute .noreturn. does not take any arguments" } +[[maybe_unused()]] int f3(); // { dg-error "attribute .maybe_unused. does not take any arguments" } +[[nodiscard()]] int f4(); // { dg-error "attribute .nodiscard. does not take any arguments" } +[[gnu::noinline()]] int f5(); // { dg-error "attribute .noinline. does not take any arguments" } +[[gnu::constructor]] int f6(); +[[gnu::constructor(101)]] int f7();