Hi!

The following testcase ICEs since my recent cxx_eval_loop_expr changes.
The problem is that the Forget saved values of SAVE_EXPRs. inside of the
loop can remove SAVE_EXPRs from new_ctx.values and if that is the last
iteration, we can also do the loop at the end of function (which has been
added there mainly to handle cases where the main loop breaks earlier)
and new_ctx.values->remove ICEs because *iter is already not in the table.

While I could e.g. add some bool whether there is anything to be removed
after the loop or not which would fix the regression part of this PR,
I believe there is a latent issue as well.  The thing is, new_ctx.save_exprs
is a hash_set, which the cxx_eval_* calls add SAVE_EXPRs to whenever they
encounter new ones, but if we don't encounter all the previously seen
SAVE_EXPRs in every iteration (e.g. some are wrapped by IF_STMT or
similar and the condition differs between iterations), I believe we can
still ICE, trying to remove a SAVE_EXPR that has been encountered by some
earlier iteration, but not the current one.

The following patch is one possible fix, only remove from new_ctx.values
what is still in there.  Bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?  Or some variant mentioned below?

Another possibility would be to save_exprs.empty (); after this
/* Forget saved values of SAVE_EXPRs.  */ loop inside of main loop (no need
to do it at the end of function when we'll just destroy it anyway), the
advantage would be that we don't really walk over SAVE_EXPRs that weren't
saved in the current iteration, disadvantage is that we need to populate the
hash_set again and again each iteration.

Yet another possibility might be to turn save_exprs into a vec, from what
I understand, we do:
    case SAVE_EXPR:
      /* Avoid evaluating a SAVE_EXPR more than once.  */
      if (tree *p = ctx->values->get (t))
        r = *p;
      else
        {
          r = cxx_eval_constant_expression (ctx, TREE_OPERAND (t, 0), false,
                                            non_constant_p, overflow_p);
          ctx->values->put (t, r);
          if (ctx->save_exprs)
            ctx->save_exprs->add (t);
        }
and thus shouldn't be adding duplicates to save_expr and if we flush it
every iteration, we could just if (ctx->save_exprs) ctx->save_exprs->safe_push 
(t);
and truncate the vector after every /* Forget saved values of SAVE_EXPRs.  */
loop in the main loop's body.  Thoughts on this?

2019-03-11  Jakub Jelinek  <ja...@redhat.com>

        PR c++/89652
        * constexpr.c (cxx_eval_loop_expr): Only remove SAVE_EXPRs that are
        still in new_ctx.values hash_map.

        * g++.dg/cpp1y/constexpr-89652.C: New test.

--- gcc/cp/constexpr.c.jj       2019-03-08 08:43:23.529496048 +0100
+++ gcc/cp/constexpr.c  2019-03-11 15:11:32.081334270 +0100
@@ -4236,7 +4236,8 @@ cxx_eval_loop_expr (const constexpr_ctx
       /* Forget saved values of SAVE_EXPRs.  */
       for (hash_set<tree>::iterator iter = save_exprs.begin();
           iter != save_exprs.end(); ++iter)
-       new_ctx.values->remove (*iter);
+       if (new_ctx.values->get (*iter))
+         new_ctx.values->remove (*iter);
 
       if (++count >= constexpr_loop_limit)
        {
@@ -4258,7 +4259,8 @@ cxx_eval_loop_expr (const constexpr_ctx
   /* Forget saved values of SAVE_EXPRs.  */
   for (hash_set<tree>::iterator iter = save_exprs.begin();
        iter != save_exprs.end(); ++iter)
-    new_ctx.values->remove (*iter);
+    if (new_ctx.values->get (*iter))
+      new_ctx.values->remove (*iter);
 
   return NULL_TREE;
 }
--- gcc/testsuite/g++.dg/cpp1y/constexpr-89652.C.jj     2019-03-11 
15:14:21.877561575 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-89652.C        2019-03-11 
15:16:11.962763933 +0100
@@ -0,0 +1,36 @@
+// PR c++/89652
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+template <typename T> constexpr auto foo (T &e) { return e.foo (); }
+template <typename T> constexpr auto bar (T &e) { return foo (e); }
+template <typename T, int N> struct A { typedef T a[N]; };
+template <typename T, unsigned long N> struct B {
+  typedef T *b;
+  typename A<T, N>::a d;
+  constexpr b foo () { return d; }
+};
+template <typename> struct C { long m; };
+struct D { long n; };
+template <typename, unsigned long> struct E {
+  B<C<int>, 1>::b p;
+  constexpr D operator* () { return {p->m}; }
+  constexpr E operator++ (int) { auto a{*this}; ++p; return a; }
+};
+template <typename T, unsigned long N>
+constexpr bool operator!= (E<T, N> a, E<T, N>) { return a.p; }
+template <unsigned long N, typename T, unsigned long M>
+constexpr auto baz (B<T, M> s, B<D, N>)
+{
+  B<D, M> t{};
+  auto q{foo (t)};
+  using u = E<T, M>;
+  auto v = u{bar (s)};
+  auto w = u{};
+  while (v != w)
+    *q++ = *v++;
+  return t;
+}
+constexpr auto a = B<C<int>, 5>{};
+auto b = B<D, 0>{};
+auto c = baz (a, b);

        Jakub

Reply via email to