c++: bad direct reference binding [PR113064]
Checks
Commit Message
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
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;
> +}
@@ -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;
new file mode 100644
@@ -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);
+}
new file mode 100644
@@ -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;
+}