On 9/6/23 18:09, Patrick Palka wrote:
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?

OK.

-- >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.


Reply via email to