On Tue, 27 Oct 2020, Jason Merrill wrote:
> On 10/26/20 5:37 PM, Patrick Palka wrote:
> > In the testcase below, we're overeagerly checking the constraints on
> > the conversion function B<int>::operator bool() as part of finding an
> > implicit conversion sequence from B<int> to const A&.
> >
> > This behavior seems to be nonconforming because according to
> > [over.match.copy] and [over.match.conv], only those conversion functions
> > which yield a type that can be converted to the target type via a
> > standard conversion sequence are candidate functions, and according to
> > [over.match.viable], only candidate functions get their constraints
> > checked.
>
> Hmm. I agree with your reading of the standard, but I'm not sure this is what
> we actually want. It's the reverse of the agreed direction for CWG2369, to
> check constraints before non-dependent conversions for function arguments, and
> goes against the more general principle that the fix for unwanted
> instantiation should be additional constraints.
>
> A different testcase would succeed before your patch, and fail after:
>
> template <class T>
> struct Error { static constexpr auto value = T::value; };
>
> struct A { A(const A&); };
>
> template <class T>
> struct B { operator Error<T>*() requires false; };
>
> template <class T>
> concept C = requires (B<T> b) { (A*)b; };
>
> static_assert(!C<int>);
>
> I've raised this on the reflector.
I see, thanks. I should note I originally hit this issue in some of the
<ranges> testcases while working on
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557237.html
to make us cache satisfaction results aggressively.
Though after some experimentation, I managed to come up with a <ranges>
testcase that misbehaves in the same way, even with GCC 10's
conservative satisfaction caching. I filed PR libstdc++/96700 and
posted a more detailed analysis of the problem.
>
> > This patch fixes this by swapping the order of the calls to
> > add_candidates and implicit_conversion in build_user_type_conversion_1
> > when the conversion function has a non-dependent result type.
> > (Conversion function templates with a dependent result type already get
> > handled appropriately in add_template_candidate, it seems.)
>
> Testcase?
>
> But yes, for dependent result types if there isn't a conversion, deduction
> will fail.
Ah, indeed. Here's an updated patch that adds to the testcase a
conversion function template for which deduction fails, to verify we
don't check its constraints either:
-- >8 --
gcc/cp/ChangeLog:
* call.c (build_user_type_conversion_1): When the conversion
function has a non-dependent result type, pre-check the
conversion sequence that would follow the conversion function.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/concepts-conv3.C: New test.
---
gcc/cp/call.c | 31 +++++++++++++++++----
gcc/testsuite/g++.dg/cpp2a/concepts-conv3.C | 19 +++++++++++++
2 files changed, 44 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-conv3.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index bd662518958..01adc5dc2ab 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4099,6 +4099,22 @@ build_user_type_conversion_1 (tree totype, tree expr,
int flags,
if (TYPE_REF_P (totype))
convflags |= LOOKUP_NO_TEMP_BIND;
+ conversion *pre_ics = NULL;
+ if (!uses_template_parms (TREE_TYPE (conv_fns)))
+ {
+ /* A conversion function is a candidate function only if it
+ yields a type that can be converted to the target type.
+ For conversion functions with non-dependent result type,
+ we check this now to discard non-candidates before calling
+ add_candidates. */
+ pre_ics = implicit_conversion (totype, TREE_TYPE (conv_fns),
+ /*expr=*/NULL_TREE,
+ /*c_cast_p=*/false, convflags,
+ complain);
+ if (!pre_ics)
+ continue;
+ }
+
old_candidates = candidates;
add_candidates (TREE_VALUE (conv_fns), first_arg, NULL, totype,
NULL_TREE, false,
@@ -4112,12 +4128,15 @@ build_user_type_conversion_1 (tree totype, tree expr,
int flags,
continue;
tree rettype = TREE_TYPE (TREE_TYPE (cand->fn));
- conversion *ics
- = implicit_conversion (totype,
- rettype,
- 0,
- /*c_cast_p=*/false, convflags,
- complain);
+ conversion *ics;
+ if (pre_ics)
+ /* No need to recompute it. */
+ ics = pre_ics;
+ else
+ ics = implicit_conversion (totype, rettype,
+ /*expr=*/NULL_TREE,
+ /*c_cast_p=*/false, convflags,
+ complain);
/* If LOOKUP_NO_TEMP_BIND isn't set, then this is
copy-initialization. In that case, "The result of the
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-conv3.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-conv3.C
new file mode 100644
index 00000000000..e0e49c737a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-conv3.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++20 } }
+
+template <class T>
+struct Error { static constexpr auto value = T::value; };
+
+struct A { A(const A&); };
+
+template <class T>
+struct B {
+ operator bool() requires Error<T>::value;
+
+ template <class U>
+ operator U*() requires Error<T>::value;
+};
+
+template <class T>
+concept C = requires (B<T> b) { A(b); };
+
+static_assert(!C<int>);
--
2.29.0.rc0
