On 3/18/25 3:12 PM, Marek Polacek wrote:
On Tue, Mar 18, 2025 at 03:05:57PM -0400, Patrick Palka wrote:
On Tue, 18 Mar 2025, Marek Polacek wrote:

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

-- >8 --
This ICE appeared with the removal of NON_DEPENDENT_EXPR.  Previously
skip_simple_arithmetic would get NON_DEPENDENT_EXPR<CAST_EXPR<>> and
since NON_DEPENDENT_EXPR is neither BINARY_CLASS_P nor UNARY_CLASS_P,
there was no problem.  But now we pass just CAST_EXPR<> and a CAST_EXPR
is a tcc_unary, so we extract its null operand and crash.

skip_simple_arithmetic is called from save_expr.  cp_save_expr already
avoids calling save_expr in a template, so that seems like an appropriate
way to fix this.

        PR c++/119344

gcc/cp/ChangeLog:

        * typeck.cc (cp_build_binary_op): Use cp_save_expr instead of save_expr.

gcc/testsuite/ChangeLog:

        * g++.dg/conversion/ptrmem10.C: New test.

LGTM.  I'm surprised the CAST_EXPR for 'T()' has TREE_SIDE_EFFECTS set
here, since it's just value initialization of scalar type.

Thanks for taking a look.  The TREE_SIDE_EFFECTS comes from:

       t = build_min (CAST_EXPR, type, parms);
       /* We don't know if it will or will not have side effects.  */
       TREE_SIDE_EFFECTS (t) = 1;

in build_functional_cast_1 where type=<record_type T>

Yep, we just don't currently bother to figure out the initialization because we don't need to in order to know the type of the cast.

The patch is OK, but let's go ahead and change the other uses of save_expr in that function as well.

---
  gcc/cp/typeck.cc                           |  4 ++--
  gcc/testsuite/g++.dg/conversion/ptrmem10.C | 14 ++++++++++++++
  2 files changed, 16 insertions(+), 2 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/conversion/ptrmem10.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 4b382b95de1..5cd1295123f 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -6019,9 +6019,9 @@ cp_build_binary_op (const op_location_t &location,
            return error_mark_node;
if (TREE_SIDE_EFFECTS (op0))
-           op0 = save_expr (op0);
+           op0 = cp_save_expr (op0);
          if (TREE_SIDE_EFFECTS (op1))
-           op1 = save_expr (op1);
+           op1 = cp_save_expr (op1);
pfn0 = pfn_from_ptrmemfunc (op0);
          pfn0 = cp_fully_fold (pfn0);
diff --git a/gcc/testsuite/g++.dg/conversion/ptrmem10.C 
b/gcc/testsuite/g++.dg/conversion/ptrmem10.C
new file mode 100644
index 00000000000..b5fc050ee81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/ptrmem10.C
@@ -0,0 +1,14 @@
+// PR c++/119344
+// { dg-do compile { target c++11 } }
+
+struct S {
+    void fn();
+};
+typedef void (S::*T)(void);
+template <T Ptr>
+struct s
+{
+  static const bool t = Ptr != T();
+};
+
+int t1 = s<&S::fn>::t;

base-commit: 82bb1890aeab275541f8d3606641e8c0cadc9659
--
2.48.1




Marek


Reply via email to