Tested on x86_64-linux, darwin, OK for trunk, and eventual backports
to fix the 126811 regression on open branches?
thanks
Iain
--- 8< ---
The initial solution proposed and landed for 126811 was to avoid calling
tsubst on constructors (motivated by a similar scheme used for constructor
elements). The intent was to avoid recursing into these where they have
NSDMIs (or other content that contains expressions that tsubst does not
expect). The concern at the time was that to handle an arbitrary set of
such conditions in tsubst would be open-ended and hard to prove complete.
However, the solution landed has just pushed that problem onto constexpr
handling and resulted in a number of reported regressions. Rather than
trying to fix these issues in constexpr, a rework of the patch for the
original (126811) problem deals with the cases in which tsubst is expected
to be visited by expressions already lowered.
One specific expected case is when tsubst is called with NULL_TREE args
and is used explicitly to rewrite the tree. We also handle cases now
where the constructor tree contains a mixture of sub-trees that still
require substitution.
In the final analysis the penalty for doing this in tsubst is small -
firstly the current set of excluded trees are (presumably) never reached
in current code (an absence of reports). The case that is reached is
actually guarded by 'checking' and therefore only enabled for trunk. We
can also short-circuit for the special case that the substitution is called
with null args - so that the original concern about complexity in tsubst
turned out to be somewhat unfounded.
The patch reverts the change previously applied for 126811 so that we do
not reopen that regression at the same time. The combination of gating
on checking and that the current reporting conditions are not reached
should make this reasonable for fixing the 126811 regression on open
branches.
My original testsuite additions were augmented with the assistance of an
LLM given the guidance that we should intend comprehensive coverage,
therefore they are partially LLM-generated.
PR c++/126811
PR c++/127395
PR c++/127439
PR c++/127573
gcc/cp/ChangeLog:
* pt.cc (tsubst_expr): Allow for CONSTRUCTORS that may have a
mixture of lowered and to-be-substituted sub-trees.
* typeck2.cc (store_init_value): Revert the gating of tsubst
for CONSTRUCTOR trees.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-scalar-init1.C: New test.
* g++.dg/cpp0x/nsdmi-template28.C: New test.
Signed-off-by: Iain Sandoe <[email protected]>
Assisted-by: Claude Opus 5 <[email protected]>
---
gcc/cp/pt.cc | 25 +++-
gcc/cp/typeck2.cc | 9 +-
.../g++.dg/cpp0x/constexpr-scalar-init1.C | 131 ++++++++++++++++++
gcc/testsuite/g++.dg/cpp0x/nsdmi-template28.C | 82 +++++++++++
4 files changed, 236 insertions(+), 11 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-scalar-init1.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/nsdmi-template28.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2b68ceb0cd2..1953a579a20 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21945,7 +21945,13 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
}
case FIX_TRUNC_EXPR:
- /* convert_like should have created an IMPLICIT_CONV_EXPR. */
+ /* convert_like should have created an IMPLICIT_CONV_EXPR, unless we
+ are on the fold path (ARGS == NULL_TREE), where we can get elements
+ that were folded outside any template (e.g an NSDMI). In those cases
+ there is nothing to substitute into them, but seeing one is not an
+ error. */
+ if (args == NULL_TREE || !instantiation_dependent_expression_p (t))
+ RETURN (t);
gcc_unreachable ();
case ADDR_EXPR:
@@ -23379,12 +23385,17 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
case CLEANUP_POINT_EXPR:
/* We shouldn't have built any of these during initial template
generation. Instead, they should be built during instantiation
- in response to the saved STMT_IS_FULL_EXPR_P setting. */
+ in response to the saved STMT_IS_FULL_EXPR_P setting - unless we
+ are on the fold path; see FIX_TRUNC_EXPR above. */
+ if (args == NULL_TREE || !instantiation_dependent_expression_p (t))
+ RETURN (t);
gcc_unreachable ();
case TARGET_EXPR:
/* TARGET_EXPR represents temporary objects and should not appear in
- templated trees. */
+ templated trees -- but see CLEANUP_POINT_EXPR above. */
+ if (args == NULL_TREE || !instantiation_dependent_expression_p (t))
+ RETURN (t);
gcc_unreachable ();
case OFFSET_REF:
@@ -23663,8 +23674,12 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
if (tree subst = objcp_tsubst_expr (t, args, complain, in_decl))
RETURN (subst);
- /* We shouldn't get here, but keep going if !flag_checking. */
- if (flag_checking)
+ /* We shouldn't get here, but keep going if !flag_checking. On the fold
+ path (ARGS == NULL_TREE) an already-folded tree has nothing to
+ substitute; see FIX_TRUNC_EXPR above. */
+ if (flag_checking
+ && args != NULL_TREE
+ && instantiation_dependent_expression_p (t))
gcc_unreachable ();
RETURN (t);
}
diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
index c7ebd080edc..08ba3da53d6 100644
--- a/gcc/cp/typeck2.cc
+++ b/gcc/cp/typeck2.cc
@@ -1072,12 +1072,9 @@ store_init_value (tree decl, tree init, vec<tree,
va_gc>** cleanups, int flags)
|| (DECL_IN_AGGR_P (decl)
&& DECL_INITIALIZED_IN_CLASS_P (decl)))
{
- /* As in massage_init_elt, do not fold a CONSTRUCTOR in a template
- it has already been folded c++/126811. */
- if (!(processing_template_decl && TREE_CODE (value) == CONSTRUCTOR))
- value = fold_non_dependent_expr (value, tf_warning_or_error,
- /*manifestly_const_eval=*/true,
- decl);
+ value = fold_non_dependent_expr (value, tf_warning_or_error,
+ /*manifestly_const_eval=*/true,
+ decl);
if (value == error_mark_node)
;
/* Diagnose a non-constant initializer for constexpr variable or
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-scalar-init1.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-scalar-init1.C
new file mode 100644
index 00000000000..5a58c6b1642
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-scalar-init1.C
@@ -0,0 +1,131 @@
+// PR c++/126811 c++/127395
+// { dg-do compile { target c++11 } }
+
+// The static_asserts ensure that we actually test the pathway.
+
+enum E { e0 = 0, e7 = 7 };
+struct C { int m; int n; void mf (); };
+using P = int *;
+using PM = int C::*; // scalar: takes the failing path
+using PMF = void (C::*) (); // RECORD_TYPE, so a different path
+using CD = _Complex double; // SCALAR_TYPE_P via ARITHMETIC_TYPE_P
+typedef int V __attribute__((vector_size (16)));
+typedef float VF __attribute__((vector_size (16)));
+
+template <int> void scalars ()
+{
+ constexpr int i0 = int{};
+ constexpr int i7 = int{7};
+ constexpr char c0 = char{};
+ constexpr bool b0 = bool{};
+ constexpr bool b1 = bool{true};
+ constexpr float f0 = float{};
+ constexpr float f2 = float{2};
+ constexpr double d0 = double{};
+ constexpr double d1 = double{1};
+ constexpr E ee = E{};
+ constexpr P pp = P{};
+ constexpr PM pm = PM{};
+ constexpr CD cd = CD{};
+
+ static_assert (i0 == 0, "int{}");
+ static_assert (i7 == 7, "int{7}");
+ static_assert (c0 == 0, "char{}");
+ static_assert (b0 == false, "bool{}");
+ static_assert (b1 == true, "bool{true}");
+ static_assert (f0 == 0.0f, "float{}");
+ static_assert (f2 == 2.0f, "float{2}");
+ static_assert (d0 == 0.0, "double{}");
+ static_assert (d1 == 1.0, "double{1}");
+ static_assert (ee == e0, "E{}");
+ static_assert (pp == nullptr, "P{}");
+ static_assert (pm == nullptr, "PM{}");
+ static_assert (__real cd == 0.0 && __imag cd == 0.0, "_Complex double{}");
+}
+
+template <int> void ptrmem ()
+{
+ constexpr PM null = PM{}; // empty: needs build_value_init,
+ constexpr PM nullp = PM{nullptr}; // not build_zero_cst
+ // one element: needs the C++
+ // conversion, not fold_convert
+ constexpr PM first = &C::m;
+ constexpr PM second = PM{&C::n};
+
+ static_assert (null == nullptr, "PM{}");
+ static_assert (nullp == nullptr, "PM{nullptr}");
+ static_assert (null != first, "null PM is not offset 0");
+ static_assert (second == &C::n, "PM{&C::n}");
+}
+
+template <int> void vectors ()
+{
+ constexpr V v0 = V{};
+ constexpr V v4 = V{1,2,3,4};
+ constexpr VF vf0 = VF{};
+ static_assert (v0[0] == 0, "V{}");
+ static_assert (v4[0] == 1 && v4[3] == 4, "V{1,2,3,4}");
+ static_assert (vf0[2] == 0.0f, "VF{}");
+}
+
+template <int> void ptrmemfunc ()
+{
+ constexpr PMF p = PMF{};
+ static_assert (p == nullptr, "PMF{}");
+}
+
+struct S { int x; double y; };
+
+template <int> void aggregates ()
+{
+ constexpr int a4[4] = {1,2,3,4};
+ constexpr int a0[4] = {};
+ constexpr S s = S{1,2.0};
+ constexpr S sz = S{};
+ static_assert (a4[0] == 1 && a4[3] == 4, "int[4] = {1,2,3,4}");
+ static_assert (a0[0] == 0 && a0[3] == 0, "int[4] = {}");
+ static_assert (s.x == 1 && s.y == 2.0, "S{1,2.0}");
+ static_assert (sz.x == 0 && sz.y == 0.0, "S{}");
+}
+
+// The fix should not touch this.
+template <int N> void dependent ()
+{
+ constexpr int i = int{N};
+ constexpr double d = double{N};
+ static_assert (i == 3, "int{N}");
+ static_assert (d == 3.0, "double{N}");
+}
+
+// or this...
+template <int> void not_constexpr ()
+{
+ int i = int{5};
+ double d = double{};
+ V v = V{1,2,3,4};
+ (void) i; (void) d; (void) v;
+}
+
+// CONSTRUCTOR should not reach the effected path.
+void nontemplate ()
+{
+ constexpr int i7 = int{7};
+ constexpr double d1 = double{1};
+ constexpr PM pm = PM{};
+ constexpr V v4 = V{1,2,3,4};
+ static_assert (i7 == 7, "int{7} outside a template");
+ static_assert (d1 == 1.0, "double{1} outside a template");
+ static_assert (pm == nullptr, "PM{} outside a template");
+ static_assert (v4[3] == 4, "V{1,2,3,4} outside a template");
+}
+
+void use ()
+{
+ scalars<0> ();
+ ptrmem<0> ();
+ vectors<0> ();
+ ptrmemfunc<0> ();
+ aggregates<0> ();
+ dependent<3> ();
+ not_constexpr<0> ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi-template28.C
b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template28.C
new file mode 100644
index 00000000000..3c74947376d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi-template28.C
@@ -0,0 +1,82 @@
+// PR c++/126811 c++/127439 c++/127593
+// { dg-do compile { target c++11 } }
+
+// This covers a CONSTRUCTOR holding both
+// * an already-folded NSDMI sub-tree
+// * an undigested IMPLICIT_CONV_EXPR
+
+struct A { double m = 1; }; // NSDMI -> FLOAT_EXPR once digested
+
+// The constructor is deliberately DECLARED BUT NOT DEFINED: that is what
+// leaves the conversion from ""
+
+struct a { constexpr a (const char *); }; // { dg-warning {inline function
'constexpr a::a.const char..' used but never defined} }
+
+struct Mix { A x; a y; };
+
+template <typename> void copy_list_init ()
+{
+ constexpr Mix m = { {}, "" };
+ (void) m;
+}
+
+template <typename> void direct_list_init ()
+{
+ constexpr Mix m{ {}, "" };
+ (void) m;
+}
+
+// Nested case.
+template <typename> void nested ()
+{
+ constexpr A x[]{ {} };
+ (void) x;
+}
+
+// opposite requirement in the same syntax.
+template <typename> void array_of_class ()
+{
+ constexpr a c[] = { "" };
+ (void) c;
+}
+
+#if __cplusplus >= 202002L
+// Designated initializers put the two halves at the SAME level with nothing
+// nested: `m` comes from the NSDMI, `y` from the conversion.
+struct Mix2 { double m = 1; a y; };
+
+template <typename> void designated ()
+{
+ constexpr Mix2 v{ .y = "" };
+ (void) v;
+}
+#endif
+
+// Value coverage for the same shape with a DEFINED constructor, so the
+// initializer really is constant-evaluated and the result can be checked.
+// This is the case a fix must not silently break while stopping the ICEs.
+struct b { constexpr b (const char *s) : p (s) {} const char *p; };
+struct MixDef { A x; b y; };
+
+template <typename> void values ()
+{
+ constexpr MixDef m{ {}, "" };
+ static_assert (m.x.m == 1.0, "NSDMI half");
+ static_assert (m.y.p[0] == 0, "converted half");
+}
+
+// The same mixture reached through a real instantiation takes a different
route
+// inside tsubst (non-null ARGS), and used to ICE there too rather than give
this
+// diagnostic.
+template <typename> void instantiated ()
+{
+ constexpr Mix m = { {}, "" }; // { dg-error "used before its
definition" }
+ (void) m;
+}
+
+void use ()
+{
+ nested<int> ();
+ instantiated<int> ();
+ values<int> ();
+}
--
2.50.1 (Apple Git-155)
