On Tue, Mar 18, 2025 at 04:19:12PM -0400, Jason Merrill wrote:
> 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.
Ok, here's what I'll push, thanks.
-- >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.
Reviewed-by: Patrick Palka <ppa...@redhat.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
---
gcc/cp/typeck.cc | 20 ++++++++++----------
gcc/testsuite/g++.dg/conversion/ptrmem10.C | 14 ++++++++++++++
2 files changed, 24 insertions(+), 10 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..c8e4441fb8b 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -5480,7 +5480,7 @@ cp_build_binary_op (const op_location_t &location,
case stv_firstarg:
{
op0 = convert (TREE_TYPE (type1), op0);
- op0 = save_expr (op0);
+ op0 = cp_save_expr (op0);
op0 = build_vector_from_val (type1, op0);
orig_type0 = type0 = TREE_TYPE (op0);
code0 = TREE_CODE (type0);
@@ -5490,7 +5490,7 @@ cp_build_binary_op (const op_location_t &location,
case stv_secondarg:
{
op1 = convert (TREE_TYPE (type0), op1);
- op1 = save_expr (op1);
+ op1 = cp_save_expr (op1);
op1 = build_vector_from_val (type0, op1);
orig_type1 = type1 = TREE_TYPE (op1);
code1 = TREE_CODE (type1);
@@ -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);
@@ -6262,8 +6262,8 @@ cp_build_binary_op (const op_location_t &location,
&& !processing_template_decl
&& sanitize_flags_p (SANITIZE_POINTER_COMPARE))
{
- op0 = save_expr (op0);
- op1 = save_expr (op1);
+ op0 = cp_save_expr (op0);
+ op1 = cp_save_expr (op1);
tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE);
instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1);
@@ -6523,14 +6523,14 @@ cp_build_binary_op (const op_location_t &location,
return error_mark_node;
if (first_complex)
{
- op0 = save_expr (op0);
+ op0 = cp_save_expr (op0);
real = cp_build_unary_op (REALPART_EXPR, op0, true, complain);
imag = cp_build_unary_op (IMAGPART_EXPR, op0, true, complain);
switch (code)
{
case MULT_EXPR:
case TRUNC_DIV_EXPR:
- op1 = save_expr (op1);
+ op1 = cp_save_expr (op1);
imag = build2 (resultcode, real_type, imag, op1);
/* Fall through. */
case PLUS_EXPR:
@@ -6543,13 +6543,13 @@ cp_build_binary_op (const op_location_t &location,
}
else
{
- op1 = save_expr (op1);
+ op1 = cp_save_expr (op1);
real = cp_build_unary_op (REALPART_EXPR, op1, true, complain);
imag = cp_build_unary_op (IMAGPART_EXPR, op1, true, complain);
switch (code)
{
case MULT_EXPR:
- op0 = save_expr (op0);
+ op0 = cp_save_expr (op0);
imag = build2 (resultcode, real_type, op0, imag);
/* Fall through. */
case PLUS_EXPR:
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: 145c90720640ec6711ed3e5aa4152bbe1ee21751
--
2.48.1
