On Thu, Mar 27, 2025 at 11:27:04AM -0400, Jason Merrill wrote:
> On 3/25/25 3:37 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14?
> >
> > -- >8 --
> > Since r15-8011 cp_build_indirect_ref_1 won't do the *&TARGET_EXPR ->
> > TARGET_EXPR folding not to change its value category. That fix is
> > correct but it made us stop extending the lifetime in this testcase,
> > causing a wrong-code issue -- extend_ref_init_temps_1 did not see
> > through the extra *& because it doesn't use a tree walk. It is not
> > hard to fix that, but there may be other places that need this
> > adjustment. :/
>
> Hmm, this suggests to me that we should revert the 117512 patch and instead
> fix it in build_over_call, perhaps by forcing 'val' to be an lvalue after
> it's built rather than relying on 'to' being an lvalue already.
Thanks for the suggestion.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14?
-- >8 --
Since r15-8011 cp_build_indirect_ref_1 won't do the *&TARGET_EXPR ->
TARGET_EXPR folding not to change its value category. That fix seems
correct but it made us stop extending the lifetime in this testcase,
causing a wrong-code issue -- extend_ref_init_temps_1 did not see
through the extra *& because it doesn't use a tree walk.
This patch reverts r15-8011 and instead handles the problem in
build_over_call by calling force_lvalue in the is_really_empty_class
case as well as in the general case.
PR c++/119383
gcc/cp/ChangeLog:
* call.cc (build_over_call): Use force_lvalue to ensure op= returns
an lvalue.
* cp-tree.h (force_lvalue): Declare.
* cvt.cc (force_lvalue): New.
* typeck.cc (cp_build_indirect_ref_1): Revert r15-8011.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/temp-extend3.C: New test.
---
gcc/cp/call.cc | 9 ++++---
gcc/cp/cp-tree.h | 1 +
gcc/cp/cvt.cc | 13 +++++++++
gcc/cp/typeck.cc | 10 +++----
gcc/testsuite/g++.dg/cpp0x/temp-extend3.C | 32 +++++++++++++++++++++++
5 files changed, 55 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/temp-extend3.C
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index c1c8987ec8b..b1469cb5a4c 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -10828,10 +10828,8 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
if (is_really_empty_class (type, /*ignore_vptr*/true))
{
/* Avoid copying empty classes, but ensure op= returns an lvalue even
- if the object argument isn't one. This isn't needed in other cases
- since MODIFY_EXPR is always considered an lvalue. */
- to = cp_build_addr_expr (to, tf_none);
- to = cp_build_indirect_ref (input_location, to, RO_ARROW, complain);
+ if the object argument isn't one. */
+ to = force_lvalue (to, complain);
val = build2 (COMPOUND_EXPR, type, arg, to);
suppress_warning (val, OPT_Wunused);
}
@@ -10852,6 +10850,9 @@ build_over_call (struct z_candidate *cand, int flags,
tsubst_flags_t complain)
tree array_type, alias_set;
arg2 = TYPE_SIZE_UNIT (as_base);
+ /* Ensure op= returns an lvalue even if the object argument isn't
+ one. */
+ to = force_lvalue (to, complain);
to = cp_stabilize_reference (to);
arg0 = cp_build_addr_expr (to, complain);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 2f2122dcf24..927f51b116b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7079,6 +7079,7 @@ extern tree convert_to_reference (tree, tree,
int, int, tree,
tsubst_flags_t);
extern tree convert_from_reference (tree);
extern tree force_rvalue (tree, tsubst_flags_t);
+extern tree force_lvalue (tree, tsubst_flags_t);
extern tree ocp_convert (tree, tree, int, int,
tsubst_flags_t);
extern tree cp_convert (tree, tree, tsubst_flags_t);
diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
index bd1f147f2c5..f663a6d08c8 100644
--- a/gcc/cp/cvt.cc
+++ b/gcc/cp/cvt.cc
@@ -575,6 +575,19 @@ force_rvalue (tree expr, tsubst_flags_t complain)
return expr;
}
+/* Force EXPR to be an lvalue, if it isn't already. */
+
+tree
+force_lvalue (tree expr, tsubst_flags_t complain)
+{
+ if (!lvalue_p (expr))
+ {
+ expr = cp_build_addr_expr (expr, complain);
+ expr = cp_build_indirect_ref (input_location, expr, RO_ARROW, complain);
+ }
+ return expr;
+}
+
/* If EXPR and ORIG are INTEGER_CSTs, return a version of EXPR that has
TREE_OVERFLOW set only if it is set in ORIG. Otherwise, return EXPR
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index c8e4441fb8b..4f4dc683b5a 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -3870,13 +3870,11 @@ cp_build_indirect_ref_1 (location_t loc, tree ptr,
ref_operator errorstring,
return error_mark_node;
}
else if (do_fold && TREE_CODE (pointer) == ADDR_EXPR
- && same_type_p (t, TREE_TYPE (TREE_OPERAND (pointer, 0)))
- /* Don't let this change the value category. '*&TARGET_EXPR'
- is an lvalue, but folding it into 'TARGET_EXPR' would turn
- it into a prvalue of class type. */
- && lvalue_p (TREE_OPERAND (pointer, 0)))
+ && same_type_p (t, TREE_TYPE (TREE_OPERAND (pointer, 0))))
/* The POINTER was something like `&x'. We simplify `*&x' to
- `x'. */
+ `x'. This change the value category: '*&TARGET_EXPR'
+ is an lvalue and folding it into 'TARGET_EXPR' turns it into
+ a prvalue of class type. */
return TREE_OPERAND (pointer, 0);
else
{
diff --git a/gcc/testsuite/g++.dg/cpp0x/temp-extend3.C
b/gcc/testsuite/g++.dg/cpp0x/temp-extend3.C
new file mode 100644
index 00000000000..3eab88d0076
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/temp-extend3.C
@@ -0,0 +1,32 @@
+// PR c++/119383
+// { dg-do run { target c++11 } }
+
+int g;
+
+struct base {
+ virtual base *clone() const = 0;
+ ~base() { }
+};
+
+struct impl : virtual base {
+ base *clone() const { return new impl; } // #1
+ impl() { ++g; }
+ ~impl() { --g; }
+};
+
+const base *
+make_a_clone ()
+{
+ const base &base = impl{}; // #2
+ return base.clone();
+}
+
+int
+main ()
+{
+ make_a_clone ();
+ // impl::impl() is called twice (#1 and #2), impl::~impl() once,
+ // at the end of make_a_clone.
+ if (g != 1)
+ __builtin_abort ();
+}
base-commit: c5d96fd5279fee47f455bd760fb0d1198a782e1e
--
2.49.0
