diff mbox series

c++: bad direct reference binding [PR113064]

Message ID 20231218195021.1244349-1-ppalka@redhat.com
State New
Headers show
Series c++: bad direct reference binding [PR113064] | expand

Commit Message

Patrick Palka Dec. 18, 2023, 7:50 p.m. UTC
Bootstrappde and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

-- >8 --

When computing a direct conversion to reference type fails and yields a
bad conversion, reference_binding incorrectly commits to that conversion
rather than attempting a conversion via a temporary.  This leads to us
rejecting the first testcase because the direct bad conversion to B&& via
the && conversion operator prevents us from considering the (good)
conversion via a temporary and the & conversion operator (and similarly
for the second more elaborate testcase).  This patch fixes this by
making reference_binding not prematurely commit to such a bad direct
conversion.  We still fall back to such a bad direct conversion if using
a temporary also fails (otherwise the diagnostic for cpp0x/explicit7.C
regresses).

	PR c++/113064

gcc/cp/ChangeLog:

	* call.cc (reference_binding): Still try a conversion via a
	temporary if a direct conversion was bad.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/rv-conv4.C: New test.
	* g++.dg/cpp0x/rv-conv5.C: New test.
---
 gcc/cp/call.cc                        | 22 ++++++++++++++++++----
 gcc/testsuite/g++.dg/cpp0x/rv-conv4.C | 16 ++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/rv-conv5.C | 23 +++++++++++++++++++++++
 3 files changed, 57 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/rv-conv4.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/rv-conv5.C

Comments

Jason Merrill Dec. 18, 2023, 9:46 p.m. UTC | #1
On 12/18/23 14:50, Patrick Palka wrote:
> Bootstrappde and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK.

> -- >8 --
> 
> When computing a direct conversion to reference type fails and yields a
> bad conversion, reference_binding incorrectly commits to that conversion
> rather than attempting a conversion via a temporary.  This leads to us
> rejecting the first testcase because the direct bad conversion to B&& via
> the && conversion operator prevents us from considering the (good)
> conversion via a temporary and the & conversion operator (and similarly
> for the second more elaborate testcase).  This patch fixes this by
> making reference_binding not prematurely commit to such a bad direct
> conversion.  We still fall back to such a bad direct conversion if using
> a temporary also fails (otherwise the diagnostic for cpp0x/explicit7.C
> regresses).
> 
> 	PR c++/113064
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (reference_binding): Still try a conversion via a
> 	temporary if a direct conversion was bad.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/rv-conv4.C: New test.
> 	* g++.dg/cpp0x/rv-conv5.C: New test.
> ---
>   gcc/cp/call.cc                        | 22 ++++++++++++++++++----
>   gcc/testsuite/g++.dg/cpp0x/rv-conv4.C | 16 ++++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/rv-conv5.C | 23 +++++++++++++++++++++++
>   3 files changed, 57 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/rv-conv4.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/rv-conv5.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 6ac87a298b2..b5d90359160 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -1739,6 +1739,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>   		   tsubst_flags_t complain)
>   {
>     conversion *conv = NULL;
> +  conversion *bad_direct_conv = nullptr;
>     tree to = TREE_TYPE (rto);
>     tree from = rfrom;
>     tree tfrom;
> @@ -1925,13 +1926,23 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>         z_candidate *cand = build_user_type_conversion_1 (rto, expr, flags,
>   							complain);
>         if (cand)
> -	return cand->second_conv;
> +	{
> +	  if (!cand->second_conv->bad_p)
> +	    return cand->second_conv;
> +
> +	  /* Direct reference binding wasn't successful and yielded
> +	     a bad conversion.  Proceed with trying to use a temporary
> +	     instead, and if that also fails then we'll return this bad
> +	     conversion rather than no conversion for sake of better
> +	     diagnostics.  */
> +	  bad_direct_conv = cand->second_conv;
> +	}
>       }
>   
>     /* From this point on, we conceptually need temporaries, even if we
>        elide them.  Only the cases above are "direct bindings".  */
>     if (flags & LOOKUP_NO_TEMP_BIND)
> -    return NULL;
> +    return bad_direct_conv ? bad_direct_conv : nullptr;
>   
>     /* [over.ics.rank]
>   
> @@ -1972,6 +1983,9 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>        there's no strictly viable candidate.  */
>     if (!maybe_valid_p && (flags & LOOKUP_SHORTCUT_BAD_CONVS))
>       {
> +      if (bad_direct_conv)
> +	return bad_direct_conv;
> +
>         conv = alloc_conversion (ck_deferred_bad);
>         conv->bad_p = true;
>         return conv;
> @@ -1995,7 +2009,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>       conv = implicit_conversion (to, from, expr, c_cast_p,
>   				flags, complain);
>     if (!conv)
> -    return NULL;
> +    return bad_direct_conv ? bad_direct_conv : nullptr;
>   
>     if (conv->user_conv_p)
>       {
> @@ -2018,7 +2032,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>   	      = reference_binding (rto, ftype, NULL_TREE, c_cast_p,
>   				   sflags, complain);
>   	    if (!new_second)
> -	      return NULL;
> +	      return bad_direct_conv ? bad_direct_conv : nullptr;
>   	    conv = merge_conversion_sequences (t, new_second);
>   	    gcc_assert (maybe_valid_p || conv->bad_p);
>   	    return conv;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-conv4.C b/gcc/testsuite/g++.dg/cpp0x/rv-conv4.C
> new file mode 100644
> index 00000000000..fea2e57531a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/rv-conv4.C
> @@ -0,0 +1,16 @@
> +// PR c++/113064
> +// { dg-do compile { target c++11 } }
> +
> +struct B { };
> +
> +struct A {
> +  operator B() &;    // #1
> +  operator B&&() &&; // #2
> +};
> +
> +void g(B&&);
> +
> +int main() {
> +  A a;
> +  g(a);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-conv5.C b/gcc/testsuite/g++.dg/cpp0x/rv-conv5.C
> new file mode 100644
> index 00000000000..dcb6fc6f76f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/rv-conv5.C
> @@ -0,0 +1,23 @@
> +// PR c++/113064
> +// { dg-do compile { target c++11 } }
> +
> +struct no_copy {
> +  no_copy() = default;
> +
> +  no_copy(const no_copy&) = delete;
> +  no_copy(no_copy&&);
> +
> +  no_copy& operator=(const no_copy&) = delete;
> +  no_copy& operator=(no_copy&&);
> +};
> +
> +struct A {
> +  operator no_copy() &;
> +  operator no_copy&&() && = delete;
> +};
> +
> +int main() {
> +  no_copy nc;
> +  A a;
> +  nc = a;
> +}
diff mbox series

Patch

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 6ac87a298b2..b5d90359160 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -1739,6 +1739,7 @@  reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
 		   tsubst_flags_t complain)
 {
   conversion *conv = NULL;
+  conversion *bad_direct_conv = nullptr;
   tree to = TREE_TYPE (rto);
   tree from = rfrom;
   tree tfrom;
@@ -1925,13 +1926,23 @@  reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
       z_candidate *cand = build_user_type_conversion_1 (rto, expr, flags,
 							complain);
       if (cand)
-	return cand->second_conv;
+	{
+	  if (!cand->second_conv->bad_p)
+	    return cand->second_conv;
+
+	  /* Direct reference binding wasn't successful and yielded
+	     a bad conversion.  Proceed with trying to use a temporary
+	     instead, and if that also fails then we'll return this bad
+	     conversion rather than no conversion for sake of better
+	     diagnostics.  */
+	  bad_direct_conv = cand->second_conv;
+	}
     }
 
   /* From this point on, we conceptually need temporaries, even if we
      elide them.  Only the cases above are "direct bindings".  */
   if (flags & LOOKUP_NO_TEMP_BIND)
-    return NULL;
+    return bad_direct_conv ? bad_direct_conv : nullptr;
 
   /* [over.ics.rank]
 
@@ -1972,6 +1983,9 @@  reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
      there's no strictly viable candidate.  */
   if (!maybe_valid_p && (flags & LOOKUP_SHORTCUT_BAD_CONVS))
     {
+      if (bad_direct_conv)
+	return bad_direct_conv;
+
       conv = alloc_conversion (ck_deferred_bad);
       conv->bad_p = true;
       return conv;
@@ -1995,7 +2009,7 @@  reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
     conv = implicit_conversion (to, from, expr, c_cast_p,
 				flags, complain);
   if (!conv)
-    return NULL;
+    return bad_direct_conv ? bad_direct_conv : nullptr;
 
   if (conv->user_conv_p)
     {
@@ -2018,7 +2032,7 @@  reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
 	      = reference_binding (rto, ftype, NULL_TREE, c_cast_p,
 				   sflags, complain);
 	    if (!new_second)
-	      return NULL;
+	      return bad_direct_conv ? bad_direct_conv : nullptr;
 	    conv = merge_conversion_sequences (t, new_second);
 	    gcc_assert (maybe_valid_p || conv->bad_p);
 	    return conv;
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-conv4.C b/gcc/testsuite/g++.dg/cpp0x/rv-conv4.C
new file mode 100644
index 00000000000..fea2e57531a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-conv4.C
@@ -0,0 +1,16 @@ 
+// PR c++/113064
+// { dg-do compile { target c++11 } }
+
+struct B { };
+
+struct A {
+  operator B() &;    // #1
+  operator B&&() &&; // #2
+};
+
+void g(B&&);
+
+int main() {
+  A a;
+  g(a);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/rv-conv5.C b/gcc/testsuite/g++.dg/cpp0x/rv-conv5.C
new file mode 100644
index 00000000000..dcb6fc6f76f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/rv-conv5.C
@@ -0,0 +1,23 @@ 
+// PR c++/113064
+// { dg-do compile { target c++11 } }
+
+struct no_copy {
+  no_copy() = default;
+
+  no_copy(const no_copy&) = delete;
+  no_copy(no_copy&&);
+
+  no_copy& operator=(const no_copy&) = delete;
+  no_copy& operator=(no_copy&&);
+};
+
+struct A {
+  operator no_copy() &;
+  operator no_copy&&() && = delete;
+};
+
+int main() {
+  no_copy nc;
+  A a;
+  nc = a;
+}