* [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
@ 2023-08-22 1:51 Patrick Palka
2023-08-23 19:45 ` Jason Merrill
0 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-08-22 1:51 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Patrick Palka
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).
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.
* g++.dg/cpp2a/concepts-nondep4a.C: New test.
---
gcc/cp/config-lang.in | 1 +
gcc/cp/pt.cc | 66 +++++++++++++++++--
gcc/cp/search.cc | 14 +++-
gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C | 21 ++++++
.../g++.dg/cpp2a/concepts-nondep4a.C | 30 +++++++++
5 files changed, 125 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-nondep4.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-nondep4a.C
diff --git a/gcc/cp/config-lang.in b/gcc/cp/config-lang.in
index a6c7883cc24..e34c392d208 100644
--- a/gcc/cp/config-lang.in
+++ b/gcc/cp/config-lang.in
@@ -52,6 +52,7 @@ gtfiles="\
\$(srcdir)/cp/name-lookup.cc \
\$(srcdir)/cp/parser.cc \$(srcdir)/cp/pt.cc \
\$(srcdir)/cp/rtti.cc \
+\$(srcdir)/cp/search.cc \
\$(srcdir)/cp/semantics.cc \
\$(srcdir)/cp/tree.cc \$(srcdir)/cp/typeck2.cc \
\$(srcdir)/cp/vtable-class-hierarchy.cc \
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index a4809f034dc..c761b73b771 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 *,
@@ -22315,7 +22315,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. */
@@ -22351,7 +22352,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;
}
@@ -22651,6 +22652,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))
{
@@ -22664,7 +22675,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:
@@ -22899,7 +22911,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;
@@ -22921,6 +22933,50 @@ 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.
+ 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);
+ }
+
+ if (CLASS_TYPE_P (type_nonref))
+ {
+ if (!COMPLETE_TYPE_P (type_nonref)
+ && CLASSTYPE_TEMPLATE_INSTANTIATION (type_nonref))
+ return unify_success (explain_p);
+
+ tree convs = lookup_conversions (type_nonref);
+ for (; convs; convs = TREE_CHAIN (convs))
+ if (TREE_CODE (TREE_VALUE (convs)) == TEMPLATE_DECL)
+ return unify_success (explain_p);
+ }
+
+ /* Checking this conversion definitely won't induce a template
+ instantiation. */
+ }
if (conv_p)
/* Avoid recalculating this in add_function_candidate. */
ok = (*conv_p
diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index cd80f285ac9..9986eec4856 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -2582,6 +2582,10 @@ lookup_conversions_r (tree binfo, int virtual_depth, int virtualness,
return my_virtualness;
}
+/* A cache of the result of lookup_conversions. */
+
+static GTY((cache)) type_tree_cache_map *conversions_cache;
+
/* Return a TREE_LIST containing all the non-hidden user-defined
conversion functions for TYPE (and its base-classes). The
TREE_VALUE of each node is the FUNCTION_DECL of the conversion
@@ -2594,12 +2598,15 @@ lookup_conversions_r (tree binfo, int virtual_depth, int virtualness,
tree
lookup_conversions (tree type)
{
- tree convs;
-
+ type = TYPE_MAIN_VARIANT (type);
complete_type (type);
if (!CLASS_TYPE_P (type) || !TYPE_BINFO (type))
return NULL_TREE;
+ if (tree *c = hash_map_safe_get (conversions_cache, type))
+ return *c;
+
+ tree convs;
lookup_conversions_r (TYPE_BINFO (type), 0, 0, NULL_TREE, NULL_TREE, &convs);
tree list = NULL_TREE;
@@ -2618,6 +2625,7 @@ lookup_conversions (tree type)
}
}
+ hash_map_safe_put<hm_ggc> (conversions_cache, type, list);
return list;
}
@@ -2798,3 +2806,5 @@ any_dependent_bases_p (tree type)
return false;
}
+
+#include "gt-cp-search.h"
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);
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-nondep4a.C b/gcc/testsuite/g++.dg/cpp2a/concepts-nondep4a.C
new file mode 100644
index 00000000000..5ee214aa495
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-nondep4a.C
@@ -0,0 +1,30 @@
+// PR c++/99599
+// { dg-do compile { target c++20 } }
+
+struct foo_tag {
+ foo_tag() = default;
+ foo_tag(int);
+ operator int();
+};
+
+struct bar_tag {
+ bar_tag() = default;
+ bar_tag(int);
+ operator int();
+};
+
+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.rc2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
2023-08-22 1:51 [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599] Patrick Palka
@ 2023-08-23 19:45 ` Jason Merrill
2023-08-24 13:31 ` Patrick Palka
0 siblings, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2023-08-23 19:45 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
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?
Also, instantiation can also happen when checking for conversion to a
pointer or reference to base class.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
2023-08-23 19:45 ` Jason Merrill
@ 2023-08-24 13:31 ` Patrick Palka
2023-08-28 22:58 ` Jason Merrill
0 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-08-24 13:31 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
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
diff --git a/gcc/cp/config-lang.in b/gcc/cp/config-lang.in
index a6c7883cc24..e34c392d208 100644
--- a/gcc/cp/config-lang.in
+++ b/gcc/cp/config-lang.in
@@ -52,6 +52,7 @@ gtfiles="\
\$(srcdir)/cp/name-lookup.cc \
\$(srcdir)/cp/parser.cc \$(srcdir)/cp/pt.cc \
\$(srcdir)/cp/rtti.cc \
+\$(srcdir)/cp/search.cc \
\$(srcdir)/cp/semantics.cc \
\$(srcdir)/cp/tree.cc \$(srcdir)/cp/typeck2.cc \
\$(srcdir)/cp/vtable-class-hierarchy.cc \
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index a4809f034dc..3c77d2466eb 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 *,
@@ -22315,7 +22315,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. */
@@ -22351,7 +22352,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;
}
@@ -22651,6 +22652,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))
{
@@ -22664,7 +22675,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:
@@ -22899,7 +22911,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;
@@ -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 ... */
+ 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;
+ }
+
+ if (CLASS_TYPE_P (parm_inner))
+ {
+ /* ... the parm's class type isn't yet instantiated, or ... */
+ if (!COMPLETE_TYPE_P (parm_inner)
+ && CLASSTYPE_TEMPLATE_INSTANTIATION (parm_inner))
+ return unify_success (explain_p);
+
+ /* constructors are considered and the parm type is a
+ non-aggregate (and therefore has user-declared
+ constructors), or ... */
+ if (!ptr_conv_p && CLASSTYPE_NON_AGGREGATE (parm_inner))
+ return unify_success (explain_p);
+ }
+
+ if (CLASS_TYPE_P (type_inner))
+ {
+ /* ... the arg's class type isn't yet instantiated, or ... */
+ if (!COMPLETE_TYPE_P (type_inner)
+ && CLASSTYPE_TEMPLATE_INSTANTIATION (type_inner))
+ return unify_success (explain_p);
+
+ /* ... conversion functions are considered and the arg's class
+ type has one that is a template or is constrained. */
+ if (!ptr_conv_p)
+ {
+ tree convs = lookup_conversions (type_inner);
+ for (; convs; convs = TREE_CHAIN (convs))
+ if (TREE_CODE (TREE_VALUE (convs)) == TEMPLATE_DECL
+ || get_constraints (TREE_VALUE (convs)) != NULL_TREE)
+ return unify_success (explain_p);
+ }
+ }
+
+ /* Otherwise, computing this conversion definitely won't induce
+ template instantiation. */
+ }
if (conv_p)
/* Avoid recalculating this in add_function_candidate. */
ok = (*conv_p
diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index cd80f285ac9..931e083580a 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -2582,6 +2582,10 @@ lookup_conversions_r (tree binfo, int virtual_depth, int virtualness,
return my_virtualness;
}
+/* A cache of the result of lookup_conversions. */
+
+static GTY((cache)) type_tree_cache_map *lookup_conversions_cache;
+
/* Return a TREE_LIST containing all the non-hidden user-defined
conversion functions for TYPE (and its base-classes). The
TREE_VALUE of each node is the FUNCTION_DECL of the conversion
@@ -2594,12 +2598,15 @@ lookup_conversions_r (tree binfo, int virtual_depth, int virtualness,
tree
lookup_conversions (tree type)
{
- tree convs;
-
+ type = TYPE_MAIN_VARIANT (type);
complete_type (type);
if (!CLASS_TYPE_P (type) || !TYPE_BINFO (type))
return NULL_TREE;
+ if (tree *c = hash_map_safe_get (lookup_conversions_cache, type))
+ return *c;
+
+ tree convs;
lookup_conversions_r (TYPE_BINFO (type), 0, 0, NULL_TREE, NULL_TREE, &convs);
tree list = NULL_TREE;
@@ -2618,6 +2625,7 @@ lookup_conversions (tree type)
}
}
+ hash_map_safe_put<hm_ggc> (lookup_conversions_cache, type, list);
return list;
}
@@ -2798,3 +2806,5 @@ any_dependent_bases_p (tree type)
return false;
}
+
+#include "gt-cp-search.h"
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.rc2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
2023-08-24 13:31 ` Patrick Palka
@ 2023-08-28 22:58 ` Jason Merrill
2023-09-06 22:00 ` Patrick Palka
2023-09-06 22:09 ` Patrick Palka
0 siblings, 2 replies; 7+ messages in thread
From: Jason Merrill @ 2023-08-28 22:58 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches
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.
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?
> + 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?
> + /* ... 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?
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
2023-08-28 22:58 ` Jason Merrill
@ 2023-09-06 22:00 ` Patrick Palka
2023-09-06 22:09 ` Patrick Palka
1 sibling, 0 replies; 7+ messages in thread
From: Patrick Palka @ 2023-09-06 22:00 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
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.
>
> 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?
>
> > + 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?
>
> > + /* ... 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?
>
> Jason
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
2023-08-28 22:58 ` Jason Merrill
2023-09-06 22:00 ` Patrick Palka
@ 2023-09-06 22:09 ` Patrick Palka
2023-09-07 18:36 ` Jason Merrill
1 sibling, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-09-06 22:09 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599]
2023-09-06 22:09 ` Patrick Palka
@ 2023-09-07 18:36 ` Jason Merrill
0 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2023-09-07 18:36 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-09-07 18:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-22 1:51 [PATCH] c++: refine CWG 2369 satisfaction vs non-dep convs [PR99599] Patrick Palka
2023-08-23 19:45 ` Jason Merrill
2023-08-24 13:31 ` Patrick Palka
2023-08-28 22:58 ` Jason Merrill
2023-09-06 22:00 ` Patrick Palka
2023-09-06 22:09 ` Patrick Palka
2023-09-07 18:36 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).