On Mon, 28 Aug 2023, Jason Merrill wrote:
> On 8/24/23 09:31, Patrick Palka wrote:
> > On Wed, 23 Aug 2023, Jason Merrill wrote:
> >
> > > On 8/21/23 21:51, Patrick Palka wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look like
> > > > a reasonable approach? I didn't observe any compile time/memory impact
> > > > of this change.
> > > >
> > > > -- >8 --
> > > >
> > > > As described in detail in the PR, CWG 2369 has the surprising
> > > > consequence of introducing constraint recursion in seemingly valid and
> > > > innocent code.
> > > >
> > > > This patch attempts to fix this surpising behavior for the majority
> > > > of problematic use cases. Rather than checking satisfaction before
> > > > _all_ non-dependent conversions, as specified by the CWG issue,
> > > > this patch makes us first check "safe" non-dependent conversions,
> > > > then satisfaction, then followed by "unsafe" non-dependent conversions.
> > > > In this case, a conversion is "safe" if computing it is guaranteed
> > > > to not induce template instantiation. This patch heuristically
> > > > determines "safety" by checking for a constructor template or conversion
> > > > function template in the (class) parm or arg types respectively.
> > > > If neither type has such a member, then computing the conversion
> > > > should not induce instantiation (modulo satisfaction checking of
> > > > non-template constructor and conversion functions I suppose).
> > > >
> > > > + /* We're checking only non-instantiating conversions.
> > > > + A conversion may instantiate only if it's to/from a
> > > > + class type that has a constructor template/conversion
> > > > + function template. */
> > > > + tree parm_nonref = non_reference (parm);
> > > > + tree type_nonref = non_reference (type);
> > > > +
> > > > + if (CLASS_TYPE_P (parm_nonref))
> > > > + {
> > > > + if (!COMPLETE_TYPE_P (parm_nonref)
> > > > + && CLASSTYPE_TEMPLATE_INSTANTIATION (parm_nonref))
> > > > + return unify_success (explain_p);
> > > > +
> > > > + tree ctors = get_class_binding (parm_nonref,
> > > > + complete_ctor_identifier);
> > > > + for (tree ctor : lkp_range (ctors))
> > > > + if (TREE_CODE (ctor) == TEMPLATE_DECL)
> > > > + return unify_success (explain_p);
> > >
> > > Today we discussed maybe checking CLASSTYPE_NON_AGGREGATE?
> >
> > Done; all dups of this PR seem to use tag types that are aggregates, so this
> > seems like a good simplification. I also made us punt if the arg type has a
> > constrained non-template conversion function.
> >
> > >
> > > Also, instantiation can also happen when checking for conversion to a
> > > pointer
> > > or reference to base class.
> >
> > Oops, I suppose we just need to strip pointer types upfront as well. The
> > !COMPLETE_TYPE_P && CLASSTYPE_TEMPLATE_INSTANTIATION tests will then make
> > sure we deem a potential derived-to-base conversion unsafe if appropriate
> > IIUC.
> >
> > How does the following look?
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs
> > [PR99599]
> >
> > PR c++/99599
> >
> > gcc/cp/ChangeLog:
> >
> > * config-lang.in (gtfiles): Add search.cc.
> > * pt.cc (check_non_deducible_conversions): Add bool parameter
> > passed down to check_non_deducible_conversion.
> > (fn_type_unification): Call check_non_deducible_conversions
> > an extra time before satisfaction with noninst_only_p=true.
> > (check_non_deducible_conversion): Add bool parameter controlling
> > whether to compute only conversions that are guaranteed to
> > not induce template instantiation.
> > * search.cc (conversions_cache): Define.
> > (lookup_conversions): Use it to cache the lookup. Improve cache
> > rate by considering TYPE_MAIN_VARIANT of the type.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp2a/concepts-nondep4.C: New test.
> > ---
> > gcc/cp/config-lang.in | 1 +
> > gcc/cp/pt.cc | 81 +++++++++++++++++--
> > gcc/cp/search.cc | 14 +++-
> > gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C | 21 +++++
> > 4 files changed, 110 insertions(+), 7 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C
> >
> > @@ -22921,6 +22933,65 @@ check_non_deducible_conversion (tree parm, tree
> > arg, unification_kind_t strict,
> > {
> > bool ok = false;
> > tree conv_arg = TYPE_P (arg) ? NULL_TREE : arg;
> > + if (conv_p && *conv_p)
> > + {
> > + /* This conversion was already computed earlier (when
> > + computing only non-instantiating conversions). */
> > + gcc_checking_assert (!noninst_only_p);
> > + return unify_success (explain_p);
> > + }
> > + if (noninst_only_p)
> > + {
> > + /* We're checking only non-instantiating conversions.
> > + Computing a conversion may induce template instantiation
> > + only if ... */
>
> Let's factor this whole block out into another function.
Sounds good.
>
> Incidentally, CWG1092 is a related problem with defaulted functions, which I
> dealt with in a stricter way: when LOOKUP_DEFAULTED we ignore a conversion
> from the parameter being copied to a non-reference-related type. As a
> follow-on, it might make sense to use this test there as well?
Interesting, I can look into that.
>
> > + tree parm_inner = non_reference (parm);
> > + tree type_inner = non_reference (type);
> > + bool ptr_conv_p = false;
> > + if (TYPE_PTR_P (parm_inner)
> > + && TYPE_PTR_P (type_inner))
> > + {
> > + parm_inner = TREE_TYPE (parm_inner);
> > + type_inner = TREE_TYPE (type_inner);
> > + ptr_conv_p = true;
> > + }
>
> I think we also want to set ptr_conv_p if the types are reference_related_p?
Ah, because in that case we know the selected conversion will always be
a derived-to-base conversion? That sounds like a nice refinement.
>
> > + /* ... conversion functions are considered and the arg's class
> > + type has one that is a template or is constrained. */
>
> Maybe just check TYPE_HAS_CONVERSION without digging into the actual
> conversions, like with CLASSTYPE_NON_AGGREGATE?
>
Sounds good, I split out the conversion function caching into a separate
patch.
Like so?
-- >8 --
Subject: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
PR c++/99599
gcc/cp/ChangeLog:
* pt.cc (check_non_deducible_conversions): Add bool parameter
passed down to check_non_deducible_conversion.
(fn_type_unification): Call check_non_deducible_conversions
an extra time before satisfaction with noninst_only_p=true.
(conversion_may_instantiate_p): Define.
(check_non_deducible_conversion): Add bool parameter controlling
whether to compute only conversions that are guaranteed to
not induce template instantiation.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/concepts-nondep4.C: New test.
---
gcc/cp/pt.cc | 86 +++++++++++++++++--
gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C | 21 +++++
2 files changed, 102 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 0790bbf948f..b528ae9a0b8 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -151,7 +151,7 @@ static tree get_partial_spec_bindings (tree, tree, tree);
static void tsubst_enum (tree, tree, tree);
static bool check_instantiated_args (tree, tree, tsubst_flags_t);
static int check_non_deducible_conversion (tree, tree, unification_kind_t, int,
- struct conversion **, bool);
+ struct conversion **, bool, bool);
static int maybe_adjust_types_for_deduction (tree, unification_kind_t,
tree*, tree*, tree);
static int type_unification_real (tree, tree, tree, const tree *,
@@ -22321,7 +22321,8 @@ pack_deducible_p (tree parm, tree fn)
static int
check_non_deducible_conversions (tree parms, const tree *args, unsigned nargs,
tree fn, unification_kind_t strict, int flags,
- struct conversion **convs, bool explain_p)
+ struct conversion **convs, bool explain_p,
+ bool noninst_only_p)
{
/* Non-constructor methods need to leave a conversion for 'this', which
isn't included in nargs here. */
@@ -22357,7 +22358,7 @@ check_non_deducible_conversions (tree parms, const tree
*args, unsigned nargs,
int lflags = conv_flags (ia, nargs, fn, arg, flags);
if (check_non_deducible_conversion (parm, arg, strict, lflags,
- conv_p, explain_p))
+ conv_p, explain_p,
noninst_only_p))
return 1;
}
@@ -22657,6 +22658,16 @@ fn_type_unification (tree fn,
deduced:
+ /* As a refinement of CWG2369, check first and foremost non-dependent
+ conversions that we know are not going to induce template instantiation
+ (PR99599). */
+ if (strict == DEDUCE_CALL
+ && incomplete
+ && check_non_deducible_conversions (parms, args, nargs, fn, strict,
flags,
+ convs, explain_p,
+ /*noninst_only_p=*/true))
+ goto fail;
+
/* CWG2369: Check satisfaction before non-deducible conversions. */
if (!constraints_satisfied_p (fn, targs))
{
@@ -22670,7 +22681,8 @@ fn_type_unification (tree fn,
as the standard says that we substitute explicit args immediately. */
if (incomplete
&& check_non_deducible_conversions (parms, args, nargs, fn, strict,
flags,
- convs, explain_p))
+ convs, explain_p,
+ /*noninst_only_p=*/false))
goto fail;
/* All is well so far. Now, check:
@@ -22897,6 +22909,60 @@ maybe_adjust_types_for_deduction (tree tparms,
return result;
}
+/* Return true if computing a conversion from FROM to TO might
+ induce template instantiation. Conversely, if this predicate
+ returns false then computing the conversion definitely won't
+ induce template instantiation. */
+
+static bool
+conversion_may_instantiate_p (tree to, tree from)
+{
+ to = non_reference (to);
+ from = non_reference (from);
+
+ bool ptr_conv_p = false;
+ if (TYPE_PTR_P (to)
+ && TYPE_PTR_P (from))
+ {
+ to = TREE_TYPE (to);
+ from = TREE_TYPE (from);
+ ptr_conv_p = true;
+ }
+
+ /* If one of the types is a not-yet-instantiated class template
+ specialization, then computing a conversion will definitely
+ perform instantiation in order to consider bases, converting
+ constructors and/or conversion functions. */
+ if ((CLASS_TYPE_P (to)
+ && !COMPLETE_TYPE_P (to)
+ && CLASSTYPE_TEMPLATE_INSTANTIATION (to))
+ || (CLASS_TYPE_P (from)
+ && !COMPLETE_TYPE_P (from)
+ && CLASSTYPE_TEMPLATE_INSTANTIATION (from)))
+ return true;
+
+ /* Converting from one pointer type to another, or between
+ reference-related types, always yields a standard conversion. */
+ if (ptr_conv_p || reference_related_p (to, from))
+ return false;
+
+ /* Converting to a non-aggregate class type will consider its
+ user-declared constructors, one of which might be a template. */
+ if (CLASS_TYPE_P (to)
+ && CLASSTYPE_NON_AGGREGATE (to))
+ return true;
+
+ /* Converting from a class type will consider its conversion functions,
+ one of which might be a template (or constrained). */
+ if (CLASS_TYPE_P (from)
+ && TYPE_HAS_CONVERSION (from))
+ return true;
+
+ /* Otherwise, computing this conversion definitely won't induce
+ template instantiation. */
+ return false;
+}
+
/* Subroutine of fn_type_unification. PARM is a function parameter of a
template which doesn't contain any deducible template parameters; check if
ARG is a suitable match for it. STRICT, FLAGS and EXPLAIN_P are as in
@@ -22905,7 +22971,7 @@ maybe_adjust_types_for_deduction (tree tparms,
static int
check_non_deducible_conversion (tree parm, tree arg, unification_kind_t strict,
int flags, struct conversion **conv_p,
- bool explain_p)
+ bool explain_p, bool noninst_only_p)
{
tree type;
@@ -22927,6 +22993,16 @@ check_non_deducible_conversion (tree parm, tree arg,
unification_kind_t strict,
{
bool ok = false;
tree conv_arg = TYPE_P (arg) ? NULL_TREE : arg;
+ if (conv_p && *conv_p)
+ {
+ /* This conversion was already computed earlier (when
+ computing only non-instantiating conversions). */
+ gcc_checking_assert (!noninst_only_p);
+ return unify_success (explain_p);
+ }
+ if (noninst_only_p
+ && conversion_may_instantiate_p (parm, type))
+ return unify_success (explain_p);
if (conv_p)
/* Avoid recalculating this in add_function_candidate. */
ok = (*conv_p
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C
new file mode 100644
index 00000000000..d26783524f7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C
@@ -0,0 +1,21 @@
+// PR c++/99599
+// { dg-do compile { target c++20 } }
+
+struct foo_tag { };
+struct bar_tag { };
+
+template <class T>
+concept fooable = requires(T it) {
+ invoke_tag(foo_tag{}, it);
+};
+
+template<class T>
+void invoke_tag(foo_tag, T in);
+
+template<fooable T>
+void invoke_tag(bar_tag, T it);
+
+int main() {
+ invoke_tag(foo_tag{}, 2);
+ invoke_tag(bar_tag{}, 2);
+}
--
2.42.0.122.g1fc548b2d6
