C++ PATCH/RFC for c++/87109, wrong overload with ref-qualifiers

[Marek Polacek <polacek@redhat.com>]

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_cadidate 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?

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-08-29  Marek Polacek  <polacek@redhat.com>

	PR c++/87109
	* call.c (add_function_candidate): Skip an rvalue ref-qualified
	candidate function when performing implicit move semantics.

	* g++.dg/cpp0x/ref-qual19.C: New test.

diff --git gcc/cp/call.c gcc/cp/call.c
index a1567026975..2911b4cfdaa 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -2194,6 +2194,7 @@ add_function_candidate (struct z_candidate **candidates,
       is_this = (i == 0 && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
 		 && ! DECL_CONSTRUCTOR_P (fn));
 
+      bool iop_to_rvalue = false;
       if (parmnode)
 	{
 	  tree parmtype = TREE_VALUE (parmnode);
@@ -2222,6 +2223,7 @@ add_function_candidate (struct z_candidate **candidates,
 		  /* The special handling of 'this' conversions in compare_ics
 		     does not apply if there is a ref-qualifier.  */
 		  is_this = false;
+		  iop_to_rvalue = rv;
 		}
 	      else
 		{
@@ -2247,6 +2249,14 @@ add_function_candidate (struct z_candidate **candidates,
 	  to_type = argtype;
 	}
 
+      /* We are performing implicit move, so the argument is an xvalue.
+	 If this candidate is an rvalue ref-qualified function, it's
+	 turned its parameter to an rvalue reference, in which case this
+	 function would be a viable candidate.  But it seems that's not
+	 what we want.  */
+      if (t && iop_to_rvalue && (flags & LOOKUP_PREFER_RVALUE))
+	t->bad_p = true;
+
       if (t && is_this)
 	t->this_p = true;
 
diff --git gcc/testsuite/g++.dg/cpp0x/ref-qual19.C gcc/testsuite/g++.dg/cpp0x/ref-qual19.C
index e69de29bb2d..b34007cd57d 100644
--- gcc/testsuite/g++.dg/cpp0x/ref-qual19.C
+++ gcc/testsuite/g++.dg/cpp0x/ref-qual19.C
@@ -0,0 +1,42 @@
+// PR c++/87109
+// { dg-do run { target c++11 } }
+
+#include <utility>
+
+struct C { int i; };
+struct A {
+  operator C() & { return { 1 }; }
+  operator C() && { return { 2 }; }
+};
+
+C
+f (A a)
+{
+  return a;
+}
+
+C
+f2 (A a)
+{
+  return std::move (a);
+}
+
+C
+f3 ()
+{
+  return A();
+}
+
+int
+main ()
+{
+  C c1 = f (A());
+  if (c1.i != 1)
+    __builtin_abort ();
+  C c2 = f2 (A());
+  if (c2.i != 2)
+    __builtin_abort ();
+  C c3 = f3 ();
+  if (c3.i != 2)
+    __builtin_abort ();
+}