Re: [PATCH] c++: bad direct reference binding [PR113064]
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 000..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 000..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&&); + +
[PATCH] c++: bad direct reference binding [PR113064]
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 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 000..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 000..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&&); +}; +