public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* C++ PATCH for c++/81429 - wrong parsing of constructor with C++11 attribute
@ 2019-08-07  7:07 Marek Polacek
  2019-08-07 16:34 ` Jason Merrill
  0 siblings, 1 reply; 3+ messages in thread
From: Marek Polacek @ 2019-08-07  7:07 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill

In this PR, we are wrongly parsing a constructor if its first parameter begins
with a C++11 attribute, e.g.:

  struct S {
    S([[maybe_unused]] int a) { }
  };

If the GNU attribute format is used instead, there's no problem.  C++11-style
attribute on a later parameter is fine also.

The problem is in cp_parser_constructor_declarator_p, in particular the code
that checks whether we're dealing with something like "S (f) (int);", which
is not a constructor.  We're checking if what comes after the first '(' is
either ')', "...", of a parameter decl.  A parameter decl can start with the
"attribute" keyword, which cp_lexer_next_token_is_decl_specifier_keyword
recognizes, but a parameter decl can also start with a C++11-style attribute,
which we forgot to realize.

The first test uses -Wunused-parameter in order to check that the attribute
is in effect.

And I also noticed that we don't issue a pedwarn about maybe_unused (C++17
attribute) in C++14: PR 91382.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I think this should
also go into 9.3.

2019-08-06  Marek Polacek  <polacek@redhat.com>

	PR c++/81429 - wrong parsing of constructor with C++11 attribute.
	* parser.c (cp_parser_constructor_declarator_p): Handle the scenario
	when a parameter declaration begins with [[attribute]].

	* g++.dg/cpp0x/gen-attrs-68.C: New test.
	* g++.dg/cpp0x/gen-attrs-69.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 79da7b52eb9..b8996c707b6 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -27855,7 +27855,9 @@ cp_parser_constructor_declarator_p (cp_parser *parser, cp_parser_flags flags,
 	  /* A parameter declaration begins with a decl-specifier,
 	     which is either the "attribute" keyword, a storage class
 	     specifier, or (usually) a type-specifier.  */
-	  && !cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer))
+	  && !cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
+	  /* A parameter declaration can also begin with [[attribute]].  */
+	  && !cp_next_tokens_can_be_std_attribute_p (parser))
 	{
 	  tree type;
 	  tree pushed_scope = NULL_TREE;
diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
new file mode 100644
index 00000000000..6bede0659db
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
@@ -0,0 +1,40 @@
+// PR c++/81429 - wrong parsing of constructor with C++11 attribute.
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wunused-parameter -Wno-pedantic" }
+
+void fn1([[maybe_unused]] int a) { }
+void fn2(int a [[maybe_unused]]) { }
+void fn3(__attribute__((unused)) int a) { }
+void fn4(int a __attribute__((unused))) { }
+
+struct S1 {
+  S1([[maybe_unused]] int a) { }
+};
+
+struct S2 {
+  S2([[maybe_unused]] int f, [[maybe_unused]] int a) { }
+};
+
+struct S3 {
+  S3(int a [[maybe_unused]]) { }
+};
+
+struct S4 {
+  S4(int f [[maybe_unused]], int a [[maybe_unused]]) { }
+};
+
+struct S5 {
+  S5(__attribute__((unused)) int a) { }
+};
+
+struct S6 {
+  S6(__attribute__((unused)) int f, __attribute__((unused)) int a) { }
+};
+
+struct S7 {
+  S7(int a __attribute__((unused))) { }
+};
+
+struct S8 {
+  S8(int f __attribute__((unused)), int a __attribute__((unused))) { }
+};
diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
new file mode 100644
index 00000000000..43173dbbdf4
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
@@ -0,0 +1,40 @@
+// PR c++/81429 - wrong parsing of constructor with C++11 attribute.
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wno-pedantic" }
+
+void fn1([[maybe_unused]] int);
+void fn2(int a [[maybe_unused]]);
+void fn3(__attribute__((unused)) int);
+void fn4(int __attribute__((unused)));
+
+struct S1 {
+  S1([[maybe_unused]] int);
+};
+
+struct S2 {
+  S2([[maybe_unused]] int, [[maybe_unused]] int);
+};
+
+struct S3 {
+  S3(int a [[maybe_unused]]);
+};
+
+struct S4 {
+  S4(int a [[maybe_unused]], int b [[maybe_unused]]);
+};
+
+struct S5 {
+  S5(__attribute__((unused)) int);
+};
+
+struct S6 {
+  S6(__attribute__((unused)) int, __attribute__((unused)) int);
+};
+
+struct S7 {
+  S7(int __attribute__((unused)));
+};
+
+struct S8 {
+  S8(int __attribute__((unused)), int __attribute__((unused)));
+};

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: C++ PATCH for c++/81429 - wrong parsing of constructor with C++11 attribute
  2019-08-07  7:07 C++ PATCH for c++/81429 - wrong parsing of constructor with C++11 attribute Marek Polacek
@ 2019-08-07 16:34 ` Jason Merrill
  2019-08-07 18:12   ` Marek Polacek
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2019-08-07 16:34 UTC (permalink / raw)
  To: Marek Polacek; +Cc: gcc-patches List

On Tue, Aug 6, 2019, 11:28 PM Marek Polacek <polacek@redhat.com> wrote:

> In this PR, we are wrongly parsing a constructor if its first parameter
> begins
> with a C++11 attribute, e.g.:
>
>   struct S {
>     S([[maybe_unused]] int a) { }
>   };
>
> If the GNU attribute format is used instead, there's no problem.
> C++11-style
> attribute on a later parameter is fine also.
>
> The problem is in cp_parser_constructor_declarator_p, in particular the
> code
> that checks whether we're dealing with something like "S (f) (int);", which
> is not a constructor.  We're checking if what comes after the first '(' is
> either ')', "...", of a parameter decl.  A parameter decl can start with
> the
> "attribute" keyword, which cp_lexer_next_token_is_decl_specifier_keyword
> recognizes, but a parameter decl can also start with a C++11-style
> attribute,
> which we forgot to realize.
>
> The first test uses -Wunused-parameter in order to check that the attribute
> is in effect.
>
> And I also noticed that we don't issue a pedwarn about maybe_unused (C++17
> attribute) in C++14: PR 91382.
>

Since unsupported attributes are supposed to be ignored, I don't think we
need to complain.

The patch is ok.

Bootstrapped/regtested on x86_64-linux, ok for trunk?  I think this should
> also go into 9.3.
>
> 2019-08-06  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/81429 - wrong parsing of constructor with C++11 attribute.
>         * parser.c (cp_parser_constructor_declarator_p): Handle the
> scenario
>         when a parameter declaration begins with [[attribute]].
>
>         * g++.dg/cpp0x/gen-attrs-68.C: New test.
>         * g++.dg/cpp0x/gen-attrs-69.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 79da7b52eb9..b8996c707b6 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -27855,7 +27855,9 @@ cp_parser_constructor_declarator_p (cp_parser
> *parser, cp_parser_flags flags,
>           /* A parameter declaration begins with a decl-specifier,
>              which is either the "attribute" keyword, a storage class
>              specifier, or (usually) a type-specifier.  */
> -         && !cp_lexer_next_token_is_decl_specifier_keyword
> (parser->lexer))
> +         && !cp_lexer_next_token_is_decl_specifier_keyword (parser->lexer)
> +         /* A parameter declaration can also begin with [[attribute]].  */
> +         && !cp_next_tokens_can_be_std_attribute_p (parser))
>         {
>           tree type;
>           tree pushed_scope = NULL_TREE;
> diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
> gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
> new file mode 100644
> index 00000000000..6bede0659db
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-68.C
> @@ -0,0 +1,40 @@
> +// PR c++/81429 - wrong parsing of constructor with C++11 attribute.
> +// { dg-do compile { target c++11 } }
> +// { dg-additional-options "-Wunused-parameter -Wno-pedantic" }
> +
> +void fn1([[maybe_unused]] int a) { }
> +void fn2(int a [[maybe_unused]]) { }
> +void fn3(__attribute__((unused)) int a) { }
> +void fn4(int a __attribute__((unused))) { }
> +
> +struct S1 {
> +  S1([[maybe_unused]] int a) { }
> +};
> +
> +struct S2 {
> +  S2([[maybe_unused]] int f, [[maybe_unused]] int a) { }
> +};
> +
> +struct S3 {
> +  S3(int a [[maybe_unused]]) { }
> +};
> +
> +struct S4 {
> +  S4(int f [[maybe_unused]], int a [[maybe_unused]]) { }
> +};
> +
> +struct S5 {
> +  S5(__attribute__((unused)) int a) { }
> +};
> +
> +struct S6 {
> +  S6(__attribute__((unused)) int f, __attribute__((unused)) int a) { }
> +};
> +
> +struct S7 {
> +  S7(int a __attribute__((unused))) { }
> +};
> +
> +struct S8 {
> +  S8(int f __attribute__((unused)), int a __attribute__((unused))) { }
> +};
> diff --git gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
> gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
> new file mode 100644
> index 00000000000..43173dbbdf4
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-69.C
> @@ -0,0 +1,40 @@
> +// PR c++/81429 - wrong parsing of constructor with C++11 attribute.
> +// { dg-do compile { target c++11 } }
> +// { dg-additional-options "-Wno-pedantic" }
> +
> +void fn1([[maybe_unused]] int);
> +void fn2(int a [[maybe_unused]]);
> +void fn3(__attribute__((unused)) int);
> +void fn4(int __attribute__((unused)));
> +
> +struct S1 {
> +  S1([[maybe_unused]] int);
> +};
> +
> +struct S2 {
> +  S2([[maybe_unused]] int, [[maybe_unused]] int);
> +};
> +
> +struct S3 {
> +  S3(int a [[maybe_unused]]);
> +};
> +
> +struct S4 {
> +  S4(int a [[maybe_unused]], int b [[maybe_unused]]);
> +};
> +
> +struct S5 {
> +  S5(__attribute__((unused)) int);
> +};
> +
> +struct S6 {
> +  S6(__attribute__((unused)) int, __attribute__((unused)) int);
> +};
> +
> +struct S7 {
> +  S7(int __attribute__((unused)));
> +};
> +
> +struct S8 {
> +  S8(int __attribute__((unused)), int __attribute__((unused)));
> +};
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: C++ PATCH for c++/81429 - wrong parsing of constructor with C++11 attribute
  2019-08-07 16:34 ` Jason Merrill
@ 2019-08-07 18:12   ` Marek Polacek
  0 siblings, 0 replies; 3+ messages in thread
From: Marek Polacek @ 2019-08-07 18:12 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches List

On Wed, Aug 07, 2019 at 12:32:54PM -0400, Jason Merrill wrote:
> On Tue, Aug 6, 2019, 11:28 PM Marek Polacek <polacek@redhat.com> wrote:
> 
> > In this PR, we are wrongly parsing a constructor if its first parameter
> > begins
> > with a C++11 attribute, e.g.:
> >
> >   struct S {
> >     S([[maybe_unused]] int a) { }
> >   };
> >
> > If the GNU attribute format is used instead, there's no problem.
> > C++11-style
> > attribute on a later parameter is fine also.
> >
> > The problem is in cp_parser_constructor_declarator_p, in particular the
> > code
> > that checks whether we're dealing with something like "S (f) (int);", which
> > is not a constructor.  We're checking if what comes after the first '(' is
> > either ')', "...", of a parameter decl.  A parameter decl can start with
> > the
> > "attribute" keyword, which cp_lexer_next_token_is_decl_specifier_keyword
> > recognizes, but a parameter decl can also start with a C++11-style
> > attribute,
> > which we forgot to realize.
> >
> > The first test uses -Wunused-parameter in order to check that the attribute
> > is in effect.
> >
> > And I also noticed that we don't issue a pedwarn about maybe_unused (C++17
> > attribute) in C++14: PR 91382.
> >
> 
> Since unsupported attributes are supposed to be ignored, I don't think we
> need to complain.

Yup, I already closed the bug.

> The patch is ok.

Thanks!

Marek

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-08-07 16:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  7:07 C++ PATCH for c++/81429 - wrong parsing of constructor with C++11 attribute Marek Polacek
2019-08-07 16:34 ` Jason Merrill
2019-08-07 18:12   ` Marek Polacek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).