Re: [PATCH] PR c++/87893 - constexpr ctor ICE on ARM.
On 1/23/19 9:26 AM, Ramana Radhakrishnan wrote: On Wed, Jan 23, 2019 at 1:54 PM Jason Merrill wrote: Since in r265788 I made cxx_eval_outermost_constant_expr more insistent that the returned value have the right type, it became more important that initialized_type be correct. These two PRs were cases of it giving the wrong answer. On ARM, a constructor returns a pointer to the object, but initialized_type should return the class type, not that pointer type. And we need to look through COMPOUND_EXPR. I tested that this fixes one of the affected testcases on ARM, and causes no regressions on x86_64-pc-linux-gnu. I haven't been able to get a cross toolchain to work properly; currently link tests are failing with undefined __sync_synchronize. Applying to trunk. rearnsha pointed this out to me - You probably need this with newlib and it looks like the patch dropped between the cracks :( https://sourceware.org/ml/newlib/2015/msg00653.html I'll try and dust this off in the coming week. Thanks, but compiling that fails for me with sync_synchronize.s: Error: unaligned opcodes detected in executable segment Would it also work to configure for a more specific target than arm-eabi? Jason
Re: [PATCH] PR c++/87893 - constexpr ctor ICE on ARM.
On Wed, Jan 23, 2019 at 1:54 PM Jason Merrill wrote:
>
> Since in r265788 I made cxx_eval_outermost_constant_expr more insistent that
> the returned value have the right type, it became more important that
> initialized_type be correct. These two PRs were cases of it giving the wrong
> answer. On ARM, a constructor returns a pointer to the object, but
> initialized_type should return the class type, not that pointer type. And we
> need to look through COMPOUND_EXPR.
>
> I tested that this fixes one of the affected testcases on ARM, and causes no
> regressions on x86_64-pc-linux-gnu. I haven't been able to get a cross
> toolchain to work properly; currently link tests are failing with
> undefined __sync_synchronize. Applying to trunk.
>
rearnsha pointed this out to me - You probably need this with newlib
and it looks like the patch dropped between the cracks :(
https://sourceware.org/ml/newlib/2015/msg00653.html
I'll try and dust this off in the coming week.
Ramana
> PR c++/88293 - ICE with comma expression.
> * constexpr.c (initialized_type): Don't shortcut non-void type.
> Handle COMPOUND_EXPR.
> (cxx_eval_outermost_constant_expr): Return early for void type.
> ---
> gcc/cp/constexpr.c| 8 +---
> gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C | 9 +
> gcc/cp/ChangeLog | 8
> 3 files changed, 22 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
>
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index ed4bbeeb157..42681416760 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -2848,9 +2848,7 @@ initialized_type (tree t)
>if (TYPE_P (t))
> return t;
>tree type = TREE_TYPE (t);
> - if (!VOID_TYPE_P (type))
> -/* No need to look deeper. */;
> - else if (TREE_CODE (t) == CALL_EXPR)
> + if (TREE_CODE (t) == CALL_EXPR)
> {
>/* A constructor call has void type, so we need to look deeper. */
>tree fn = get_function_named_in_call (t);
> @@ -2858,6 +2856,8 @@ initialized_type (tree t)
> && DECL_CXX_CONSTRUCTOR_P (fn))
> type = DECL_CONTEXT (fn);
> }
> + else if (TREE_CODE (t) == COMPOUND_EXPR)
> +return initialized_type (TREE_OPERAND (t, 1));
>else if (TREE_CODE (t) == AGGR_INIT_EXPR)
> type = TREE_TYPE (AGGR_INIT_EXPR_SLOT (t));
>return cv_unqualified (type);
> @@ -5061,6 +5061,8 @@ cxx_eval_outermost_constant_expr (tree t, bool
> allow_non_constant,
>
>tree type = initialized_type (t);
>tree r = t;
> + if (VOID_TYPE_P (type))
> +return t;
>if (AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type))
> {
>/* In C++14 an NSDMI can participate in aggregate initialization,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
> b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
> new file mode 100644
> index 000..9dd1299ddf4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
> @@ -0,0 +1,9 @@
> +// PR c++/88293
> +// { dg-do compile { target c++11 } }
> +
> +struct A
> +{
> + constexpr A () { }
> +};
> +
> +const A = (A (), A ());
> diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
> index 111782aeaba..20a54719578 100644
> --- a/gcc/cp/ChangeLog
> +++ b/gcc/cp/ChangeLog
> @@ -1,3 +1,11 @@
> +2019-01-21 Jason Merrill
> +
> + PR c++/87893 - constexpr ctor ICE on ARM.
> + PR c++/88293 - ICE with comma expression.
> + * constexpr.c (initialized_type): Don't shortcut non-void type.
> + Handle COMPOUND_EXPR.
> + (cxx_eval_outermost_constant_expr): Return early for void type.
> +
> 2019-01-21 Jakub Jelinek
>
> PR c++/88949
>
> base-commit: feb90a0dd6fc4e12786dce8338f716253d50b545
> --
> 2.20.1
>
[PATCH] PR c++/87893 - constexpr ctor ICE on ARM.
Since in r265788 I made cxx_eval_outermost_constant_expr more insistent that
the returned value have the right type, it became more important that
initialized_type be correct. These two PRs were cases of it giving the wrong
answer. On ARM, a constructor returns a pointer to the object, but
initialized_type should return the class type, not that pointer type. And we
need to look through COMPOUND_EXPR.
I tested that this fixes one of the affected testcases on ARM, and causes no
regressions on x86_64-pc-linux-gnu. I haven't been able to get a cross
toolchain to work properly; currently link tests are failing with
undefined __sync_synchronize. Applying to trunk.
PR c++/88293 - ICE with comma expression.
* constexpr.c (initialized_type): Don't shortcut non-void type.
Handle COMPOUND_EXPR.
(cxx_eval_outermost_constant_expr): Return early for void type.
---
gcc/cp/constexpr.c| 8 +---
gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C | 9 +
gcc/cp/ChangeLog | 8
3 files changed, 22 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index ed4bbeeb157..42681416760 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -2848,9 +2848,7 @@ initialized_type (tree t)
if (TYPE_P (t))
return t;
tree type = TREE_TYPE (t);
- if (!VOID_TYPE_P (type))
-/* No need to look deeper. */;
- else if (TREE_CODE (t) == CALL_EXPR)
+ if (TREE_CODE (t) == CALL_EXPR)
{
/* A constructor call has void type, so we need to look deeper. */
tree fn = get_function_named_in_call (t);
@@ -2858,6 +2856,8 @@ initialized_type (tree t)
&& DECL_CXX_CONSTRUCTOR_P (fn))
type = DECL_CONTEXT (fn);
}
+ else if (TREE_CODE (t) == COMPOUND_EXPR)
+return initialized_type (TREE_OPERAND (t, 1));
else if (TREE_CODE (t) == AGGR_INIT_EXPR)
type = TREE_TYPE (AGGR_INIT_EXPR_SLOT (t));
return cv_unqualified (type);
@@ -5061,6 +5061,8 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
tree type = initialized_type (t);
tree r = t;
+ if (VOID_TYPE_P (type))
+return t;
if (AGGREGATE_TYPE_P (type) || VECTOR_TYPE_P (type))
{
/* In C++14 an NSDMI can participate in aggregate initialization,
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
new file mode 100644
index 000..9dd1299ddf4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-comma1.C
@@ -0,0 +1,9 @@
+// PR c++/88293
+// { dg-do compile { target c++11 } }
+
+struct A
+{
+ constexpr A () { }
+};
+
+const A = (A (), A ());
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 111782aeaba..20a54719578 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2019-01-21 Jason Merrill
+
+ PR c++/87893 - constexpr ctor ICE on ARM.
+ PR c++/88293 - ICE with comma expression.
+ * constexpr.c (initialized_type): Don't shortcut non-void type.
+ Handle COMPOUND_EXPR.
+ (cxx_eval_outermost_constant_expr): Return early for void type.
+
2019-01-21 Jakub Jelinek
PR c++/88949
base-commit: feb90a0dd6fc4e12786dce8338f716253d50b545
--
2.20.1
