diff mbox series

c++: Fix C++11 attribute propagation [PR106712]

Message ID 20220826230150.328756-1-polacek@redhat.com
State New, archived
Headers
Series c++: Fix C++11 attribute propagation [PR106712] |

Commit Message

Marek Polacek Aug. 26, 2022, 11:01 p.m. UTC 
  When we have

  [[noreturn]] int fn1 [[nodiscard]](), fn2();

"noreturn" should apply to both fn1 and fn2 but "nodiscard" only to fn1:
[dcl.pre]/3: "The attribute-specifier-seq appertains to each of
the entities declared by the declarators of the init-declarator-list."
[dcl.spec.general]: "The attribute-specifier-seq affects the type
only for the declaration it appears in, not other declarations involving
the same type."

As Ed Catmur correctly analyzed, this is because, for the test above,
we call start_decl with prefix_attributes=noreturn, but this line:

  attributes = attr_chainon (attributes, prefix_attributes);

results in attributes == prefix_attributes, because chainon sees
that attributes is null so it just returns prefix_attributes.  Then
in grokdeclarator we reach

  *attrlist = attr_chainon (*attrlist, declarator->std_attributes);

which modifies prefix_attributes so now it's "noreturn, nodiscard"
and so fn2 is wrongly marked nodiscard as well.  Fixed by copying
prefix_attributes.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/106712

gcc/cp/ChangeLog:

	* decl.cc (start_decl): Copy prefix_attributes.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/gen-attrs-77.C: New test.
---
 gcc/cp/decl.cc                            |  3 ++-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C


base-commit: 390f94eee1ae694901f896ac45bfb148f8126baa

Comments

Jason Merrill Aug. 29, 2022, 5:32 p.m. UTC | #1 
On 8/26/22 19:01, Marek Polacek wrote:
> When we have
> 
>    [[noreturn]] int fn1 [[nodiscard]](), fn2();
> 
> "noreturn" should apply to both fn1 and fn2 but "nodiscard" only to fn1:
> [dcl.pre]/3: "The attribute-specifier-seq appertains to each of
> the entities declared by the declarators of the init-declarator-list."
> [dcl.spec.general]: "The attribute-specifier-seq affects the type
> only for the declaration it appears in, not other declarations involving
> the same type."
> 
> As Ed Catmur correctly analyzed, this is because, for the test above,
> we call start_decl with prefix_attributes=noreturn, but this line:
> 
>    attributes = attr_chainon (attributes, prefix_attributes);
> 
> results in attributes == prefix_attributes, because chainon sees
> that attributes is null so it just returns prefix_attributes.  Then
> in grokdeclarator we reach
> 
>    *attrlist = attr_chainon (*attrlist, declarator->std_attributes);
> 
> which modifies prefix_attributes so now it's "noreturn, nodiscard"
> and so fn2 is wrongly marked nodiscard as well.  Fixed by copying
> prefix_attributes.

How about reversing the order of arguments to the call in 
grokdeclarator, so that the prefix attributes can remain shared at the 
end of the list?

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/106712
> 
> gcc/cp/ChangeLog:
> 
> 	* decl.cc (start_decl): Copy prefix_attributes.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/gen-attrs-77.C: New test.
> ---
>   gcc/cp/decl.cc                            |  3 ++-
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C | 17 +++++++++++++++++
>   2 files changed, 19 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index d46a347a6c7..9fa80b926d5 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -5566,7 +5566,8 @@ start_decl (const cp_declarator *declarator,
>     *pushed_scope_p = NULL_TREE;
>   
>     if (prefix_attributes != error_mark_node)
> -    attributes = attr_chainon (attributes, prefix_attributes);
> +    /* Don't let grokdeclarator modify prefix_attributes.  */
> +    attributes = attr_chainon (attributes, copy_list (prefix_attributes));
>   
>     decl = grokdeclarator (declarator, declspecs, NORMAL, initialized,
>   			 &attributes);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C
> new file mode 100644
> index 00000000000..2c41c62f33b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C
> @@ -0,0 +1,17 @@
> +// PR c++/106712
> +// { dg-do compile { target c++11 } }
> +
> +[[noreturn]] int f1 [[nodiscard]](), f2 ();
> +[[nodiscard]] int f3 (), f4 ();
> +int f5 [[nodiscard]](), f6 ();
> +
> +int
> +main ()
> +{
> +  f1 (); // { dg-warning "ignoring" }
> +  f2 ();
> +  f3 (); // { dg-warning "ignoring" }
> +  f4 (); // { dg-warning "ignoring" }
> +  f5 (); // { dg-warning "ignoring" }
> +  f6 ();
> +}
> 
> base-commit: 390f94eee1ae694901f896ac45bfb148f8126baa
diff mbox series

Patch

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index d46a347a6c7..9fa80b926d5 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -5566,7 +5566,8 @@  start_decl (const cp_declarator *declarator,
   *pushed_scope_p = NULL_TREE;
 
   if (prefix_attributes != error_mark_node)
-    attributes = attr_chainon (attributes, prefix_attributes);
+    /* Don't let grokdeclarator modify prefix_attributes.  */
+    attributes = attr_chainon (attributes, copy_list (prefix_attributes));
 
   decl = grokdeclarator (declarator, declspecs, NORMAL, initialized,
 			 &attributes);
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C
new file mode 100644
index 00000000000..2c41c62f33b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-77.C
@@ -0,0 +1,17 @@ 
+// PR c++/106712
+// { dg-do compile { target c++11 } }
+
+[[noreturn]] int f1 [[nodiscard]](), f2 ();
+[[nodiscard]] int f3 (), f4 ();
+int f5 [[nodiscard]](), f6 ();
+
+int
+main ()
+{
+  f1 (); // { dg-warning "ignoring" }
+  f2 ();
+  f3 (); // { dg-warning "ignoring" }
+  f4 (); // { dg-warning "ignoring" }
+  f5 (); // { dg-warning "ignoring" }
+  f6 ();
+}