Re: C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
On 9/11/19 4:15 PM, Marek Polacek wrote:
This ICEs since r267272 - more location wrapper nodes, but not because we can't
cope with new location wrappers, but because that commit introduced a call to
maybe_constant_value in cp_build_array_ref. In this testcase we call it with
f (VIEW_CONVERT_EXPR("BAR")) ? 1 : 0
argument and that crashes in fold_convert because we end up trying to convert
"BAR" of type const char[4] to const char * when evaluating the call. At this
point, decay_conversion hasn't turned the argument into (const char *) "BAR"
yet.
The ICE doesn't occur without :?, because then the call will be wrapped in
NON_DEPENDENT_EXPR and constexpr throws its hands (I'm anthropomorphizing) up
when it encounters such an expression.
I noticed that build_non_dependent_expr doesn't wrap op0 of ?: in N_D_E. This
is so since r70606 -- Nathan, is there a reason not to do it? Doing it fixes
this problem.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-09-11 Marek Polacek
PR c++/91740 - ICE with constexpr call and ?: in ARRAY_REF.
* pt.c (build_non_dependent_expr): Call build_non_dependent_expr for
the first operand.
* g++.dg/cpp1y/var-templ63.C: New test.
diff --git gcc/cp/pt.c gcc/cp/pt.c
index c5915a5ecd0..775389d8245 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr)
if (TREE_CODE (expr) == COND_EXPR)
return build3 (COND_EXPR,
TREE_TYPE (expr),
- TREE_OPERAND (expr, 0),
+ build_non_dependent_expr (TREE_OPERAND (expr, 0)),
(TREE_OPERAND (expr, 1)
? build_non_dependent_expr (TREE_OPERAND (expr, 1))
: build_non_dependent_expr (TREE_OPERAND (expr, 0))),
OK. I wonder why this code copies op0 for the ?: extension rather than
leave it null?
Jason
Re: C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
On Thu, Sep 12, 2019 at 11:08:43AM +0200, Paolo Carlini wrote: > Hi again, > > On 12/09/19 11:03, Paolo Carlini wrote: > > Hi, > > > > On 11/09/19 23:15, Marek Polacek wrote: > > > --- gcc/cp/pt.c > > > +++ gcc/cp/pt.c > > > @@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr) > > > if (TREE_CODE (expr) == COND_EXPR) > > > return build3 (COND_EXPR, > > > TREE_TYPE (expr), > > > - TREE_OPERAND (expr, 0), > > > + build_non_dependent_expr (TREE_OPERAND (expr, 0)), > > > (TREE_OPERAND (expr, 1) > > > ? build_non_dependent_expr (TREE_OPERAND (expr, 1)) > > > : build_non_dependent_expr (TREE_OPERAND (expr, 0))), > > > > Looks like we would end up unnecessarily calling > > build_non_dependent_expr three times instead of two: probably is very > > cheap, probably the code is cleaner this way but I'm a little annoyed at > > this anyway, for the record ;) > > Sorry, I misread the code: normally TREE_OPERAND (expr, 1) isn't NULL_TREE > thus we are fine. And I forgot to mention that build_x_conditional_expr has 6743 ifexp = build_non_dependent_expr (ifexp); 6744 if (op1) 6745 op1 = build_non_dependent_expr (op1); 6746 op2 = build_non_dependent_expr (op2); which means my fix should make more sense. -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Re: C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
Hi again, On 12/09/19 11:03, Paolo Carlini wrote: Hi, On 11/09/19 23:15, Marek Polacek wrote: --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr) if (TREE_CODE (expr) == COND_EXPR) return build3 (COND_EXPR, TREE_TYPE (expr), - TREE_OPERAND (expr, 0), + build_non_dependent_expr (TREE_OPERAND (expr, 0)), (TREE_OPERAND (expr, 1) ? build_non_dependent_expr (TREE_OPERAND (expr, 1)) : build_non_dependent_expr (TREE_OPERAND (expr, 0))), Looks like we would end up unnecessarily calling build_non_dependent_expr three times instead of two: probably is very cheap, probably the code is cleaner this way but I'm a little annoyed at this anyway, for the record ;) Sorry, I misread the code: normally TREE_OPERAND (expr, 1) isn't NULL_TREE thus we are fine. Paolo.
Re: C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
Hi, On 11/09/19 23:15, Marek Polacek wrote: --- gcc/cp/pt.c +++ gcc/cp/pt.c @@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr) if (TREE_CODE (expr) == COND_EXPR) return build3 (COND_EXPR, TREE_TYPE (expr), - TREE_OPERAND (expr, 0), + build_non_dependent_expr (TREE_OPERAND (expr, 0)), (TREE_OPERAND (expr, 1) ? build_non_dependent_expr (TREE_OPERAND (expr, 1)) : build_non_dependent_expr (TREE_OPERAND (expr, 0))), Looks like we would end up unnecessarily calling build_non_dependent_expr three times instead of two: probably is very cheap, probably the code is cleaner this way but I'm a little annoyed at this anyway, for the record ;) Cheers, Paolo.
C++ PATCH for c++/91740 - ICE with constexpr call and ?: in ARRAY_REF
This ICEs since r267272 - more location wrapper nodes, but not because we can't
cope with new location wrappers, but because that commit introduced a call to
maybe_constant_value in cp_build_array_ref. In this testcase we call it with
f (VIEW_CONVERT_EXPR("BAR")) ? 1 : 0
argument and that crashes in fold_convert because we end up trying to convert
"BAR" of type const char[4] to const char * when evaluating the call. At this
point, decay_conversion hasn't turned the argument into (const char *) "BAR"
yet.
The ICE doesn't occur without :?, because then the call will be wrapped in
NON_DEPENDENT_EXPR and constexpr throws its hands (I'm anthropomorphizing) up
when it encounters such an expression.
I noticed that build_non_dependent_expr doesn't wrap op0 of ?: in N_D_E. This
is so since r70606 -- Nathan, is there a reason not to do it? Doing it fixes
this problem.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-09-11 Marek Polacek
PR c++/91740 - ICE with constexpr call and ?: in ARRAY_REF.
* pt.c (build_non_dependent_expr): Call build_non_dependent_expr for
the first operand.
* g++.dg/cpp1y/var-templ63.C: New test.
diff --git gcc/cp/pt.c gcc/cp/pt.c
index c5915a5ecd0..775389d8245 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -26709,7 +26709,7 @@ build_non_dependent_expr (tree expr)
if (TREE_CODE (expr) == COND_EXPR)
return build3 (COND_EXPR,
TREE_TYPE (expr),
- TREE_OPERAND (expr, 0),
+ build_non_dependent_expr (TREE_OPERAND (expr, 0)),
(TREE_OPERAND (expr, 1)
? build_non_dependent_expr (TREE_OPERAND (expr, 1))
: build_non_dependent_expr (TREE_OPERAND (expr, 0))),
diff --git gcc/testsuite/g++.dg/cpp1y/var-templ63.C
gcc/testsuite/g++.dg/cpp1y/var-templ63.C
new file mode 100644
index 000..a65f53b2963
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/var-templ63.C
@@ -0,0 +1,5 @@
+// PR c++/91740 - ICE with constexpr call and ?: in ARRAY_REF.
+// { dg-do compile { target c++14 } }
+
+constexpr bool f(const char*) { return true; }
+template const char c = "FOO"[f("BAR") ? 1 : 0];
