On Tue, Sep 4, 2018 at 3:02 PM, Marek Polacek <pola...@redhat.com> wrote: > On Thu, Aug 30, 2018 at 09:22:26AM -0400, Jason Merrill wrote: >> On Wed, Aug 29, 2018 at 8:03 PM, Marek Polacek <pola...@redhat.com> wrote: >> > I've now gotten to the point where I question the validity of this PR, so >> > it's >> > probably a good time to stop and ask for some advice. >> > >> > As discussed in >> > <https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01607.html>, we >> > choose the wrong overload for f1: >> > >> > struct C { }; >> > struct A { >> > operator C() &; >> > operator C() &&; >> > }; >> > >> > C f1(A a) >> > { >> > return a; // should call operator C()&, but calls operator C()&& >> > } >> > >> > Since we're returning a local variable, we know it's about to be destroyed, >> > so even though it's got a name, not for very much longer, so we activate >> > move >> > semantics. So we perform overload resolution with 'a' turned into >> > *NON_LVALUE_EXPR<(A&) &a>, an xvalue. We need to convert 'a' from A to C, >> > which is taking place in build_user_type_conversion_1. It will see two >> > cadidates: >> > >> > A::operator C() && >> > A::operator C() & >> > >> > when adding these candidates in add_function_candidate we process the >> > ref-qualifiers by tweaking the type of the implicit object parameter by >> > turning >> > it into a reference type. Then we create an implicit conversion sequence >> > for converting the type of the argument to the type of the parameter, >> > so A to A&. That succeeds in the first case (an xvalue binding to an >> > rvalue >> > reference) but fails in the second case (an xvalue binding to an lvalue >> > reference). And thus we end up using the first overload. >> > >> > But why is this invalid, again? [class.copy.elision] says "or if the type >> > of >> > the first parameter of the selected constructor is not an rvalue reference >> > to >> > the object's type (possibly cv-qualified), overload resolution is performed >> > again, considering the object as an lvalue." but I don't see how that >> > applies >> > here. (Constructors can't have ref-qualifiers anyway.) >> > >> > Thoughts? >> >> Where that rule comes in is when we choose the constructor for C: >> since we've already called operator C()&&, we choose C(C&&), which >> does not have a first parameter of "rvalue reference to cv A", so it >> should be rejected. > > I see. Turned out there were two problems: we weren't getting into the > if (flags & LOOKUP_PREFER_RVALUE) block in build_over_call; this I fixed > by setting rvaluedness_matches_p in build_user_type_conversion_1 for ck_rvalue > if appropriate. > > And then the constructor argument check failed to check the returned object's > type. The tricky part is getting the type. I realized we can go to the > beginning of the conversion sequence and extract u.expr. But we can't simply > look at its type, if it's DECL_CONV_FN_P, we need to dig deeper. You'll see > that I don't handle other things, and I don't know if I need to. I tried to > handle arbitrary expressions, but it evolved into something that just seemed > too fragile. > > E.g. for this testcase u.expr will look like: > TARGET_EXPR <D.2335, A::operator C ((struct A *) NON_LVALUE_EXPR <(struct A > &) &a>)> > and taking its type would yield "struct C". I actually think that returning > error_mark_node when we see any DECL_CONV_FN_P would work just as well. > > Can you think of something better than this?
Hmm, I think we could push that back even farther, and bail out where you're already changing build_user_type_conversion_1; if we're doing this two-step initialization, then we aren't going to end up with a suitable constructor. Jason
