On Mon, Dec 02, 2019 at 01:48:46PM -0500, Jason Merrill wrote:
> On 11/29/19 6:33 PM, Jakub Jelinek wrote:
> > Hi!
> >
> > The changed code in build_new_op_1 ICEs on the following testcase,
> > because conv is user_conv_p with kind == ck_ambig, for which next_conversion
> > returns NULL. It seems in other spots where for user_conv_p we are walking
> > the conversion chain we also don't assume there must be ck_user, so this
> > patch just uses the first conv if ck_user is not found (so that the previous
> > diagnostics about ambiguous conversion is emitted).
>
> It seems like various places could use a function to strip down to the first
> ck_user, ck_ambig, ck_list, ck_aggr, or ck_identity conversion encountered:
> source_type, the user-defined conversion comparision in compare_ics, and now
> here. Mind doing that refactoring? Maybe call it
> strip_standard_conversion.
In neither of the spots it is exactly equivalent to what the code was doing
before (or what the patched build_new_op_1 did), but this is an area I know
next to nothing about, so I've just tried to type into patch what you wrote
above. Do you mean something like below (didn't see regressions in make
check-c++-all, but haven't tried full bootstrap/regtest yet)?
2019-12-02 Jason Merrill <ja...@redhat.com>
Jakub Jelinek <ja...@redhat.com>
PR c++/92705
* call.c (strip_standard_conversion): New function.
(build_new_op_1): Use it for user_conv_p.
(compare_ics): Likewise.
(source_type): Likewise.
* g++.dg/conversion/ambig4.C: New test.
--- gcc/cp/call.c.jj 2019-11-30 00:15:46.298953538 +0100
+++ gcc/cp/call.c 2019-12-02 23:02:48.494379961 +0100
@@ -865,6 +865,22 @@ next_conversion (conversion *conv)
return conv->u.next;
}
+/* Strip to the first ck_user, ck_ambig, ck_list, ck_aggr or ck_identity
+ encountered. */
+
+static conversion *
+strip_standard_conversion (conversion *conv)
+{
+ while (conv
+ && conv->kind != ck_user
+ && conv->kind != ck_ambig
+ && conv->kind != ck_list
+ && conv->kind != ck_aggr
+ && conv->kind != ck_identity)
+ conv = next_conversion (conv);
+ return conv;
+}
+
/* Subroutine of build_aggr_conv: check whether CTOR, a braced-init-list,
is a valid aggregate initializer for array type ATYPE. */
@@ -6370,8 +6386,7 @@ build_new_op_1 (const op_location_t &loc
conv = cand->convs[0];
if (conv->user_conv_p)
{
- while (conv->kind != ck_user)
- conv = next_conversion (conv);
+ conv = strip_standard_conversion (conv);
arg1 = convert_like (conv, arg1, complain);
}
@@ -6380,8 +6395,7 @@ build_new_op_1 (const op_location_t &loc
conv = cand->convs[1];
if (conv->user_conv_p)
{
- while (conv->kind != ck_user)
- conv = next_conversion (conv);
+ conv = strip_standard_conversion (conv);
arg2 = convert_like (conv, arg2, complain);
}
}
@@ -6391,8 +6405,7 @@ build_new_op_1 (const op_location_t &loc
conv = cand->convs[2];
if (conv->user_conv_p)
{
- while (conv->kind != ck_user)
- conv = next_conversion (conv);
+ conv = strip_standard_conversion (conv);
arg3 = convert_like (conv, arg3, complain);
}
}
@@ -10538,17 +10551,8 @@ compare_ics (conversion *ics1, conversio
if (ics1->user_conv_p || ics1->kind == ck_list
|| ics1->kind == ck_aggr || ics2->kind == ck_aggr)
{
- conversion *t1;
- conversion *t2;
-
- for (t1 = ics1; t1 && t1->kind != ck_user; t1 = next_conversion (t1))
- if (t1->kind == ck_ambig || t1->kind == ck_aggr
- || t1->kind == ck_list)
- break;
- for (t2 = ics2; t2 && t2->kind != ck_user; t2 = next_conversion (t2))
- if (t2->kind == ck_ambig || t2->kind == ck_aggr
- || t2->kind == ck_list)
- break;
+ conversion *t1 = strip_standard_conversion (ics1);
+ conversion *t2 = strip_standard_conversion (ics2);
if (!t1 || !t2 || t1->kind != t2->kind)
return 0;
@@ -10956,14 +10960,7 @@ compare_ics (conversion *ics1, conversio
static tree
source_type (conversion *t)
{
- for (;; t = next_conversion (t))
- {
- if (t->kind == ck_user
- || t->kind == ck_ambig
- || t->kind == ck_identity)
- return t->type;
- }
- gcc_unreachable ();
+ return strip_standard_conversion (t)->type;
}
/* Note a warning about preferring WINNER to LOSER. We do this by storing
--- gcc/testsuite/g++.dg/conversion/ambig4.C.jj 2019-12-02 22:52:24.407017475
+0100
+++ gcc/testsuite/g++.dg/conversion/ambig4.C 2019-12-02 22:52:24.407017475
+0100
@@ -0,0 +1,14 @@
+// PR c++/92705
+// { dg-do compile }
+
+struct A {};
+struct B {};
+struct C { operator B * (); }; // { dg-message "candidate" }
+struct D { operator B * (); }; // { dg-message "candidate" }
+struct E : C, D { operator A * (); };
+
+void
+foo (E e, int B::* pmf)
+{
+ int i = e->*pmf; // { dg-error "is ambiguous" }
+}
Jakub
