c++: Fix ICE with CONSTRUCTOR flags verification [PR93559]

Message ID 20200205211757.2335536-1-polacek@redhat.com
State New
Headers show
Series
  • c++: Fix ICE with CONSTRUCTOR flags verification [PR93559]
Related show

Commit Message

Marek Polacek Feb. 5, 2020, 9:17 p.m.
Since reshape_init_array_1 can now reuse a single constructor for
an array of non-aggregate type, we might run into a scenario where
we reuse a constructor with TREE_SIDE_EFFECTS.  This broke this test
because we have something like { { expr } } and we try to reshape it,
so we recurse on the inner CONSTRUCTOR, reuse an existing CONSTRUCTOR
with TREE_SIDE_EFFECTS, and then ICE on the discrepancy because the
outermost CONSTRUCTOR doesn't have TREE_SIDE_EFFECTS.  In this case
EXPR was a call to an operator function so TREE_SIDE_EFFECTS should
be set.  Naturally one would want to fix this by calling
recompute_constructor_flags in an appropriate place so that the flags
on the CONSTRUCTORs match.  The appropriate place would be at the end
of reshape_init, but this breaks initlist109.C: there we are dealing
with { { TARGET_EXPR <{}> } } where the outermost { } is TREE_CONSTANT
but the inner { } is not, so recompute_constructor_flags would clear
the constant flag in the outermost { }.  Seems resonable but it upsets
check_initializer which then complains about "non-constant in-class
initialization invalid for static member".  TARGET_EXPRs are always
created with TREE_SIDE_EFFECTS on, but that is mutually exclusive
with TREE_CONSTANT.  So we're in a bind.

Fixed by not reusing a CONSTRUCTOR that has TREE_SIDE_EFFECTS; in the
grand scheme of things it isn't measurable: it only affects ~3 tests
in the testsuite.

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

	PR c++/93559 - ICE with CONSTRUCTOR flags verification.
	* decl.c (reshape_init_array_1): Don't reuse a CONSTRUCTOR with
	TREE_SIDE_EFFECTS.

	* g++.dg/cpp0x/initlist119.C: New test.
	* g++.dg/cpp0x/initlist120.C: New test.
---
 gcc/cp/decl.c                            |  4 +++-
 gcc/testsuite/g++.dg/cpp0x/initlist119.C | 15 +++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/initlist120.C | 16 ++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist119.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist120.C


base-commit: 91bc3c98851670360dfcd312399c3ba35fb50231
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Comments

Jason Merrill Feb. 5, 2020, 9:27 p.m. | #1
On Wed, Feb 5, 2020 at 4:18 PM Marek Polacek <polacek@redhat.com> wrote:

> Since reshape_init_array_1 can now reuse a single constructor for

> an array of non-aggregate type, we might run into a scenario where

> we reuse a constructor with TREE_SIDE_EFFECTS.  This broke this test

> because we have something like { { expr } } and we try to reshape it,

> so we recurse on the inner CONSTRUCTOR, reuse an existing CONSTRUCTOR

> with TREE_SIDE_EFFECTS, and then ICE on the discrepancy because the

> outermost CONSTRUCTOR doesn't have TREE_SIDE_EFFECTS.  In this case

> EXPR was a call to an operator function so TREE_SIDE_EFFECTS should

> be set.  Naturally one would want to fix this by calling

> recompute_constructor_flags in an appropriate place so that the flags

> on the CONSTRUCTORs match.  The appropriate place would be at the end

> of reshape_init, but this breaks initlist109.C: there we are dealing

> with { { TARGET_EXPR <{}> } } where the outermost { } is TREE_CONSTANT

> but the inner { } is not, so recompute_constructor_flags would clear

> the constant flag in the outermost { }.  Seems resonable but it upsets

> check_initializer which then complains about "non-constant in-class

> initialization invalid for static member".  TARGET_EXPRs are always

> created with TREE_SIDE_EFFECTS on, but that is mutually exclusive

> with TREE_CONSTANT.  So we're in a bind.

>

> Fixed by not reusing a CONSTRUCTOR that has TREE_SIDE_EFFECTS; in the

> grand scheme of things it isn't measurable: it only affects ~3 tests

> in the testsuite.

>


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


OK.

        PR c++/93559 - ICE with CONSTRUCTOR flags verification.
>         * decl.c (reshape_init_array_1): Don't reuse a CONSTRUCTOR with

>         TREE_SIDE_EFFECTS.

>

>         * g++.dg/cpp0x/initlist119.C: New test.

>         * g++.dg/cpp0x/initlist120.C: New test.

> ---

>  gcc/cp/decl.c                            |  4 +++-

>  gcc/testsuite/g++.dg/cpp0x/initlist119.C | 15 +++++++++++++++

>  gcc/testsuite/g++.dg/cpp0x/initlist120.C | 16 ++++++++++++++++

>  3 files changed, 34 insertions(+), 1 deletion(-)

>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist119.C

>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist120.C

>

> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c

> index 859fd1bb931..d90007ba475 100644

> --- a/gcc/cp/decl.c

> +++ b/gcc/cp/decl.c

> @@ -5989,7 +5989,9 @@ reshape_init_array_1 (tree elt_type, tree max_index,

> reshape_iter *d,

>    /* The initializer for an array is always a CONSTRUCTOR.  If this is the

>       outermost CONSTRUCTOR and the element type is non-aggregate, we

> don't need

>       to build a new one.  */

> -  bool reuse = first_initializer_p && !CP_AGGREGATE_TYPE_P (elt_type);

> +  bool reuse = (first_initializer_p

> +               && !CP_AGGREGATE_TYPE_P (elt_type)

> +               && !TREE_SIDE_EFFECTS (first_initializer_p));

>    if (reuse)

>      new_init = first_initializer_p;

>    else

> diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist119.C

> b/gcc/testsuite/g++.dg/cpp0x/initlist119.C

> new file mode 100644

> index 00000000000..80f391f64c7

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/initlist119.C

> @@ -0,0 +1,15 @@

> +// PR c++/93559 - ICE with CONSTRUCTOR flags verification.

> +// { dg-do compile { target c++11 } }

> +

> +struct E { int d[10]; };

> +

> +struct S {

> +  constexpr int operator()(char) { return 42; }

> +};

> +

> +template <typename> struct X {

> +  constexpr static E foo(S s) { return {{s(1)}}; }

> +};

> +

> +S s;

> +static_assert((X<S>::foo(s), 1), "");

> diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist120.C

> b/gcc/testsuite/g++.dg/cpp0x/initlist120.C

> new file mode 100644

> index 00000000000..8d03166e197

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/initlist120.C

> @@ -0,0 +1,16 @@

> +// PR c++/93559 - ICE with CONSTRUCTOR flags verification.

> +// { dg-do compile { target c++11 } }

> +

> +struct F { int d[10]; };

> +struct E { F f; };

> +

> +struct S {

> +  constexpr int operator()(char) { return 42; }

> +};

> +

> +template <typename> struct X {

> +  constexpr static E foo(S s) { return {{{s(1)}}}; }

> +};

> +

> +S s;

> +static_assert((X<S>::foo(s), 1), "");

>

> base-commit: 91bc3c98851670360dfcd312399c3ba35fb50231

> --

> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

>

>

Patch

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 859fd1bb931..d90007ba475 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5989,7 +5989,9 @@  reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d,
   /* The initializer for an array is always a CONSTRUCTOR.  If this is the
      outermost CONSTRUCTOR and the element type is non-aggregate, we don't need
      to build a new one.  */
-  bool reuse = first_initializer_p && !CP_AGGREGATE_TYPE_P (elt_type);
+  bool reuse = (first_initializer_p
+		&& !CP_AGGREGATE_TYPE_P (elt_type)
+		&& !TREE_SIDE_EFFECTS (first_initializer_p));
   if (reuse)
     new_init = first_initializer_p;
   else
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist119.C b/gcc/testsuite/g++.dg/cpp0x/initlist119.C
new file mode 100644
index 00000000000..80f391f64c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist119.C
@@ -0,0 +1,15 @@ 
+// PR c++/93559 - ICE with CONSTRUCTOR flags verification.
+// { dg-do compile { target c++11 } }
+
+struct E { int d[10]; };
+
+struct S {
+  constexpr int operator()(char) { return 42; }
+};
+
+template <typename> struct X {
+  constexpr static E foo(S s) { return {{s(1)}}; }
+};
+
+S s;
+static_assert((X<S>::foo(s), 1), "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist120.C b/gcc/testsuite/g++.dg/cpp0x/initlist120.C
new file mode 100644
index 00000000000..8d03166e197
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist120.C
@@ -0,0 +1,16 @@ 
+// PR c++/93559 - ICE with CONSTRUCTOR flags verification.
+// { dg-do compile { target c++11 } }
+
+struct F { int d[10]; };
+struct E { F f; };
+
+struct S {
+  constexpr int operator()(char) { return 42; }
+};
+
+template <typename> struct X {
+  constexpr static E foo(S s) { return {{{s(1)}}}; }
+};
+
+S s;
+static_assert((X<S>::foo(s), 1), "");