Re: [PATCH] c++: Fix cp_fold_non_odr_use_1 [PR123557,PR123738]

Jason Merrill Mon, 26 Jan 2026 04:50:02 -0800

On 1/24/26 8:00 PM, Nathaniel Shead wrote:

On Sat, Jan 24, 2026 at 10:49:17PM +1100, Nathaniel Shead wrote:

On Sat, Jan 24, 2026 at 09:41:51PM +1100, Nathaniel Shead wrote:

On Sat, Jan 24, 2026 at 04:57:44PM +0800, Jason Merrill wrote:

On 1/24/26 4:41 PM, Nathaniel Shead wrote:

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?


-- >8 --

This fixes two issues with cp_fold_non_odr_use_1:

- When called to fold away a reference, it considers the use to not be
    an lvalue and so 'maybe_constant_value' folds all the way through the
    nested INDIRECT_REF to a prvalue.  Fixed by forcing the expression
    to be an ADDR_EXPR before calling 'maybe_constant_value', and then
    folding it away afterwards.


For the REFERENCE_REF case it would make sense to pass its op0 to
fold_non_odr rather than the ref itself; we already convert_from_reference
the result.


Right, makes sense; thanks.  Here's an updated patch that does this
which passes bootstrap and regtest on x86_64-pc-linux-gnu, OK for trunk?


Actually, ignore this: I just noticed that this fails with modules in
some cases with wrong-looking GIMPLE; for cases like

   export module M;
   static const int& ref = 123;
   constexpr int foo() { return ref; }

we no longer fold 'ref' to 123 but instead return

   '(int)*(const int&)&_ZGRL3ref_'

which then can fail calls because the local symbol is not defined in
importers.  Even my original patch will break on this case; I'll need to
think more about what to do here.


Sorry, again misread: my original patch does actually work here.  I'll
need to sleep soon but I'll take another look at why that one worked and
this one didn't; it seems to be that the issue does come down to whether
cxx_eval_outermost_constant_eval considers us to be in an lvalue or
rvalue context, changing the way that it folds constant values.

I think _outermost_ is fine. The problem is that with this patch wefirst _fold_non_odr_ in cp_fold, which returns the above, so when wethen call _fold_non_odr_ again in cp_fold_maybe_rvalue the argument nolonger looks like a use of a variable.

So we want to avoid the double _fold_non_odr_. Maybe move the rval &&non_odr handliing above the cp_fold in _maybe_rvalue_?

If my hack of wrapping in an ADDR_EXPR before passing to
maybe_constant_value is not appropriate I'll look into maybe making
another entrypoint to cxx_eval_outermost_constant_eval that does start
with vc_glvalue perhaps.

-- >8 --

This fixes two issues with cp_fold_non_odr_use_1:

- When called to fold away a reference, it considers the use to not be
   an lvalue and so 'maybe_constant_value' folds all the way through the
   nested INDIRECT_REF to a prvalue.  Fixed by folding the op0 of the
   INDIRECT_REF rather than the ref itself.

- When used to fold away the initializing expression for an INIT_EXPR,
   it doesn't mark any new TARGET_EXPRs as eliding.  Fixed by reapplying
   'set_target_expr_eliding' for the initializer of an INIT_EXPR if it
   got modified during ff_only_non_odr walk.

        PR c++/123557
        PR c++/123738

gcc/cp/ChangeLog:

        * cp-gimplify.cc (cp_fold): Pass op0 to cp_fold_non_odr_use_1
        when folding a reference.  Reapply set_target_expr_eliding on
        the initializing expression of an INIT_EXPR.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp0x/constexpr-ice22.C: New test.
        * g++.dg/cpp2a/constexpr-ref2.C: New test.

Signed-off-by: Nathaniel Shead <[email protected]>
Reviewed-by: Jason Merrill <[email protected]>
---
  gcc/cp/cp-gimplify.cc                        | 12 ++++++++----
  gcc/testsuite/g++.dg/cpp0x/constexpr-ice22.C | 14 ++++++++++++++
  gcc/testsuite/g++.dg/cpp2a/constexpr-ref2.C  | 19 +++++++++++++++++++
  3 files changed, 41 insertions(+), 4 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ice22.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-ref2.C

diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 3c0f415fa87..1189c100527 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -3411,9 +3411,9 @@ cp_fold (tree x, fold_flags_t flags)
         used as lvalues.  */
        if ((flags & ff_only_non_odr) && REFERENCE_REF_P (x))
        {
-         tree r = cp_fold_non_odr_use_1 (x);
-         if (r != x)
-           return convert_from_reference (cp_fold (r, flags));
+         op0 = cp_fold_non_odr_use_1 (TREE_OPERAND (x, 0));
+         if (op0 != TREE_OPERAND (x, 0))
+           return convert_from_reference (cp_fold (op0, flags));
        }
        goto unary;

@@ -3560,7 +3560,11 @@ cp_fold (tree x, fold_flags_t flags)

          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 = build2_loc (loc, code, TREE_TYPE (x), op0, op1);
+           {
+             if (code == INIT_EXPR && op1 != TREE_OPERAND (x, 1))
+               set_target_expr_eliding (op1);
+             x = build2_loc (loc, code, TREE_TYPE (x), op0, op1);
+           }
          break;
        }

diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ice22.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-ice22.C

new file mode 100644
index 00000000000..900c8053a92
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ice22.C
@@ -0,0 +1,14 @@
+// PR c++/123557
+// { dg-do compile { target c++11 } }
+
+struct a {
+  a() = default;
+  a(a const &) = default;
+  a(a &&);
+};
+struct c {
+  c();
+  a e;
+};
+constexpr a b;
+c::c() : e(b) {}
diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-ref2.C 
b/gcc/testsuite/g++.dg/cpp2a/constexpr-ref2.C
new file mode 100644
index 00000000000..659beed3a25
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-ref2.C
@@ -0,0 +1,19 @@
+// PR c++/123738
+// { dg-do compile { target c++20 } }
+
+struct OStringLiteral {
+    int str = 0;
+};
+
+template<auto L> struct OStringHolder {
+    static constexpr auto & literal = L;
+};
+
+struct OString {
+    template<auto L> constexpr OString(OStringHolder<L> const &):
+        p(&OStringHolder<L>::literal.str) {}
+    int const * p;
+};
+
+
+constexpr OString s = OStringHolder<OStringLiteral{}>{};
--
2.51.0

Re: [PATCH] c++: Fix cp_fold_non_odr_use_1 [PR123557,PR123738]

Reply via email to