Re: [PATCH 1/2] c++: Don't call fold from cp_fold if one of the operands is an error_mark [PR118525]

Jason Merrill Tue, 21 Jan 2025 14:03:04 -0800

On 1/16/25 7:10 PM, Andrew Pinski wrote:

While adding a new match pattern, g++.dg/cpp2a/consteval36.C started to ICE and 
that was
because we would call fold even if one of the operands of the comparison was an 
error_mark_node.
I found a new testcase which also ICEs before this patch too so show the issue 
was latent.


So there is code in cp_fold to avoid calling fold when one of the operands 
become error_mark_node
but with the addition of consteval, the replacement of an invalid call is 
replaced before the call
to cp_fold and there is no way to pop up the error_mark. So this patch changes 
the current code to
check if the operands of the expression are error_mark_node before checking if 
the folded operand
is different from the previous one.

Hmm, I'm surprised the fold functions don't return error_mark_node forerror operand. But the patch is OK, thanks.

Bootstrapped and tested on x86_64-linux-gnu.

        PR c++/118525

gcc/cp/ChangeLog:

        * cp-gimplify.cc (cp_fold): Check operands of unary, binary, 
cond/vec_cond
        and array_ref for error_mark before checking if the operands had 
changed.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp2a/consteval38.C: New test.

Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
---
  gcc/cp/cp-gimplify.cc                    | 99 ++++++++++--------------
  gcc/testsuite/g++.dg/cpp2a/consteval38.C | 11 +++
  2 files changed, 53 insertions(+), 57 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval38.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index c7074b00cef..4ec3de13008 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -3005,19 +3005,16 @@ cp_fold (tree x, fold_flags_t flags)
        loc = EXPR_LOCATION (x);
        op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops, flags);

- if (code == CONVERT_EXPR

+      if (op0 == error_mark_node)
+       x = error_mark_node;
+      else if (code == CONVERT_EXPR
          && SCALAR_TYPE_P (TREE_TYPE (x))
          && op0 != void_node)
        /* During parsing we used convert_to_*_nofold; re-convert now using the
           folding variants, since fold() doesn't do those transformations.  */
        x = fold (convert (TREE_TYPE (x), op0));
        else if (op0 != TREE_OPERAND (x, 0))
-       {
-         if (op0 == error_mark_node)
-           x = error_mark_node;
-         else
-           x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
-       }
+       x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
        else
        x = fold (x);

@@ -3087,20 +3084,17 @@ cp_fold (tree x, fold_flags_t flags)

        op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops, flags);

finish_unary:

-      if (op0 != TREE_OPERAND (x, 0))
+      if (op0 == error_mark_node)
+       x = error_mark_node;
+      else if (op0 != TREE_OPERAND (x, 0))
        {
-         if (op0 == error_mark_node)
-           x = error_mark_node;
-         else
+         x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
+         if (code == INDIRECT_REF
+             && (INDIRECT_REF_P (x) || TREE_CODE (x) == MEM_REF))
            {
-             x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
-             if (code == INDIRECT_REF
-                 && (INDIRECT_REF_P (x) || TREE_CODE (x) == MEM_REF))
-               {
-                 TREE_READONLY (x) = TREE_READONLY (org_x);
-                 TREE_SIDE_EFFECTS (x) = TREE_SIDE_EFFECTS (org_x);
-                 TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x);
-               }
+             TREE_READONLY (x) = TREE_READONLY (org_x);
+             TREE_SIDE_EFFECTS (x) = TREE_SIDE_EFFECTS (org_x);
+             TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x);
            }
        }
        else
@@ -3190,13 +3184,10 @@ cp_fold (tree x, fold_flags_t flags)
                                        op0, op1);
        }

- if (op0 != TREE_OPERAND (x, 0) || op1 != TREE_OPERAND (x, 1))

-       {
-         if (op0 == error_mark_node || op1 == error_mark_node)
-           x = error_mark_node;
-         else
-           x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1);
-       }
+      if (op0 == error_mark_node || op1 == error_mark_node)
+       x = error_mark_node;
+      else if (op0 != TREE_OPERAND (x, 0) || op1 != TREE_OPERAND (x, 1))
+       x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1);
        else
        x = fold (x);

@@ -3268,17 +3259,14 @@ cp_fold (tree x, fold_flags_t flags)

            }
        }

- if (op0 != TREE_OPERAND (x, 0)

-         || op1 != TREE_OPERAND (x, 1)
-         || op2 != TREE_OPERAND (x, 2))
-       {
-         if (op0 == error_mark_node
-             || op1 == error_mark_node
-             || op2 == error_mark_node)
-           x = error_mark_node;
-         else
-           x = fold_build3_loc (loc, code, TREE_TYPE (x), op0, op1, op2);
-       }
+      if (op0 == error_mark_node
+         || op1 == error_mark_node
+         || op2 == error_mark_node)
+       x = error_mark_node;
+      else if (op0 != TREE_OPERAND (x, 0)
+              || op1 != TREE_OPERAND (x, 1)
+              || op2 != TREE_OPERAND (x, 2))
+       x = fold_build3_loc (loc, code, TREE_TYPE (x), op0, op1, op2);
        else
        x = fold (x);

@@ -3462,14 +3450,14 @@ cp_fold (tree x, fold_flags_t flags)

        FOR_EACH_VEC_SAFE_ELT (elts, i, p)
          {
            tree op = cp_fold (p->value, flags);
-           if (op != p->value)
+           if (op == error_mark_node)
+             {
+               x = error_mark_node;
+               vec_free (nelts);
+               break;
+             }
+           else if (op != p->value)
              {
-               if (op == error_mark_node)
-                 {
-                   x = error_mark_node;
-                   vec_free (nelts);
-                   break;
-                 }
                if (nelts == NULL)
                  nelts = elts->copy ();
                (*nelts)[i].value = op;
@@ -3516,23 +3504,20 @@ cp_fold (tree x, fold_flags_t flags)
        op2 = cp_fold (TREE_OPERAND (x, 2), flags);
        op3 = cp_fold (TREE_OPERAND (x, 3), flags);

- if (op0 != TREE_OPERAND (x, 0)

+      if (op0 == error_mark_node
+         || op1 == error_mark_node
+         || op2 == error_mark_node
+         || op3 == error_mark_node)
+       x = error_mark_node;
+      else if (op0 != TREE_OPERAND (x, 0)
          || op1 != TREE_OPERAND (x, 1)
          || op2 != TREE_OPERAND (x, 2)
          || op3 != TREE_OPERAND (x, 3))
        {
-         if (op0 == error_mark_node
-             || op1 == error_mark_node
-             || op2 == error_mark_node
-             || op3 == error_mark_node)
-           x = error_mark_node;
-         else
-           {
-             x = build4_loc (loc, code, TREE_TYPE (x), op0, op1, op2, op3);
-             TREE_READONLY (x) = TREE_READONLY (org_x);
-             TREE_SIDE_EFFECTS (x) = TREE_SIDE_EFFECTS (org_x);
-             TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x);
-           }
+         x = build4_loc (loc, code, TREE_TYPE (x), op0, op1, op2, op3);
+         TREE_READONLY (x) = TREE_READONLY (org_x);
+         TREE_SIDE_EFFECTS (x) = TREE_SIDE_EFFECTS (org_x);
+         TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x);
        }

x = fold (x);

diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval38.C 
b/gcc/testsuite/g++.dg/cpp2a/consteval38.C
new file mode 100644
index 00000000000..cc9f7ba8c82
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval38.C
@@ -0,0 +1,11 @@
+// PR c++/118525
+// { dg-do compile { target c++20 } }
+
+// Make sure we don't ICE
+consteval int id (int i) { return i; }
+
+void
+g (int i)
+{
+    1 ? 1 : id (i) ^ 1; // { dg-error "call to consteval function|'i' is not a 
constant expression" }
+}

Re: [PATCH 1/2] c++: Don't call fold from cp_fold if one of the operands is an error_mark [PR118525]

Reply via email to