On Thu, Apr 23, 2020 at 02:48:38PM -0400, Jason Merrill wrote:
> On 4/22/20 11:27 PM, Marek Polacek wrote:
> > This test is rejected with a bogus "use of deleted function" error
> > starting with r225705 whereby convert_like_real/ck_base no longer
> > sets LOOKUP_ONLYCONVERTING for user_conv_p conversions. This does
> > not seem to be always correct. To recap, when we have something like
> > T t = x where T is a class type and the type of x is not T or derived
> > from T, we perform copy-initialization, something like:
> > 1. choose a user-defined conversion to convert x to T, the result is
> > a prvalue,
> > 2. use this prvalue to direct-initialize t.
> >
> > In the second step, explicit constructors should be considered, since
> > we're direct-initializing. This is what r225705 fixed.
> >
> > In this PR we are dealing with the first step, I think, where explicit
> > constructors should be skipped. [over.match.copy] says "The converting
> > constructors of T are candidate functions" which clearly eliminates
> > explicit constructors. But we also have to copy-initialize the argument
> > we are passing to such a converting constructor, and here we should
> > disregard explicit constructors too.
> >
> > In this testcase we have
> >
> > V v = m;
> >
> > and we choose V::V(M) to convert m to V. But we wrongly choose
> > the explicit M::M<M&>(M&) to copy-initialize the argument; it's
> > a better match for a non-const lvalue than the implicit M::M(const M&)
> > but because it's explicit, we shouldn't use it.
> >
> > When convert_like is processing the ck_user conversion -- the convfn is
> > V::V(M) -- it can see that cand->flags contains LOOKUP_ONLYCONVERTING,
> > but then when we're in build_over_call for this convfn, we have no way
> > to pass the flag to convert_like for the argument 'm', because convert_like
> > doesn't take flags. So I've resorted to setting need_temporary_p in
> > a ck_rvalue, thus far unused, to signal that we're only interested in
> > non-explicit constructors.
> >
> > LOOKUP_COPY_PARM looks relevant, but again, it's a LOOKUP_* flag, so
> > can't pass it to convert_like. DR 899 also seemed related, but that
> > deals with direct-init contexts only.
> >
> > Does this make sense?
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > PR c++/90320
> > * call.c (standard_conversion): Set need_temporary_p if FLAGS demands
> > LOOKUP_ONLYCONVERTING.
> > (convert_like_real) <case ck_base>: If a ck_rvalue has
> > need_temporary_p set, or LOOKUP_ONLYCONVERTING into FLAGS.
> >
> > * g++.dg/cpp0x/explicit13.C: New test.
> > ---
> > gcc/cp/call.c | 24 +++++++++++++++++-------
> > gcc/testsuite/g++.dg/cpp0x/explicit13.C | 14 ++++++++++++++
> > 2 files changed, 31 insertions(+), 7 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/explicit13.C
> >
> > diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> > index c58231601c9..d802f1a0c2f 100644
> > --- a/gcc/cp/call.c
> > +++ b/gcc/cp/call.c
> > @@ -92,10 +92,10 @@ struct conversion {
> > language standards, e.g. disregarding pointer qualifiers or
> > converting integers to pointers. */
> > BOOL_BITFIELD bad_p : 1;
> > - /* If KIND is ck_ref_bind ck_base_conv, true to indicate that a
> > + /* If KIND is ck_ref_bind or ck_base, true to indicate that a
> > temporary should be created to hold the result of the
> > - conversion. If KIND is ck_ambig or ck_user, true means force
> > - copy-initialization. */
> > + conversion. If KIND is ck_ambig, ck_rvalue, or ck_user, true means
> > + force copy-initialization. */
> > BOOL_BITFIELD need_temporary_p : 1;
> > /* If KIND is ck_ptr or ck_pmem, true to indicate that a conversion
> > from a pointer-to-derived to pointer-to-base is being performed. */
> > @@ -1252,6 +1252,8 @@ standard_conversion (tree to, tree from, tree expr,
> > bool c_cast_p,
> > if (flags & LOOKUP_PREFER_RVALUE)
> > /* Tell convert_like_real to set LOOKUP_PREFER_RVALUE. */
> > conv->rvaluedness_matches_p = true;
> > + if (flags & LOOKUP_ONLYCONVERTING)
> > + conv->need_temporary_p = true;
>
> Presumably we want the same thing for ck_base?
We actually already set need_temporary_p for ck_base in standard_conversion:
1529 conv->need_temporary_p = !(flags & LOOKUP_NO_TEMP_BIND);
> > @@ -7654,10 +7656,18 @@ convert_like_real (conversion *convs, tree expr,
> > tree fn, int argnum,
> > destination [is treated as direct-initialization]. [dcl.init] */
> > flags = LOOKUP_NORMAL;
> > if (convs->user_conv_p)
> > - /* This conversion is being done in the context of a user-defined
> > - conversion (i.e. the second step of copy-initialization), so
> > - don't allow any more. */
> > - flags |= LOOKUP_NO_CONVERSION;
> > + {
> > + /* This conversion is being done in the context of a user-defined
> > + conversion (i.e. the second step of copy-initialization), so
> > + don't allow any more. */
> > + flags |= LOOKUP_NO_CONVERSION;
> > + /* But we might also be performing a conversion of the argument
> > + to the user-defined conversion, i.e., not a conversion of the
> > + result of the user-defined conversion. In which case we skip
> > + explicit constructors. */
> > + if (convs->kind == ck_rvalue && convs->need_temporary_p)
> > + flags |= LOOKUP_ONLYCONVERTING;
> > + }
> > else
> > flags |= LOOKUP_ONLYCONVERTING;
>
> And then here set LOOKUP_ONLYCONVERTING only based on need_temporary_p
> regardless of user_conv_p?
Could do. How's this patch? I've included a test with a derived-to-base
conversion to exercise ck_base.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? Maybe 9 too
after a while.
-- >8 --
This test is rejected with a bogus "use of deleted function" error
starting with r225705 whereby convert_like_real/ck_base no longer
sets LOOKUP_ONLYCONVERTING for user_conv_p conversions. This does
not seem to be always correct. To recap, when we have something like
T t = x where T is a class type and the type of x is not T or derived
from T, we perform copy-initialization, something like:
1. choose a user-defined conversion to convert x to T, the result is
a prvalue,
2. use this prvalue to direct-initialize t.
In the second step, explicit constructors should be considered, since
we're direct-initializing. This is what r225705 fixed.
In this PR we are dealing with the first step, I think, where explicit
constructors should be skipped. [over.match.copy] says "The converting
constructors of T are candidate functions" which clearly eliminates
explicit constructors. But we also have to copy-initialize the argument
we are passing to such a converting constructor, and here we should
disregard explicit constructors too.
In this testcase we have
V v = m;
and we choose V::V(M) to convert m to V. But we wrongly choose
the explicit M::M<M&>(M&) to copy-initialize the argument; it's
a better match for a non-const lvalue than the implicit M::M(const M&)
but because it's explicit, we shouldn't use it.
When convert_like is processing the ck_user conversion -- the convfn is
V::V(M) -- it can see that cand->flags contains LOOKUP_ONLYCONVERTING,
but then when we're in build_over_call for this convfn, we have no way
to pass the flag to convert_like for the argument 'm', because convert_like
doesn't take flags. So I've resorted to setting need_temporary_p in
a ck_rvalue, thus far unused, to signal that we're only interested in
non-explicit constructors.
LOOKUP_COPY_PARM looks relevant, but again, it's a LOOKUP_* flag, so
can't pass it to convert_like. DR 899 also seemed related, but that
deals with direct-init contexts only.
PR c++/90320
* call.c (standard_conversion): Set need_temporary_p if FLAGS demands
LOOKUP_ONLYCONVERTING.
(convert_like_real) <case ck_base>: If need_temporary_p is set, or
LOOKUP_ONLYCONVERTING into FLAGS.
* g++.dg/cpp0x/explicit13.C: New test.
* g++.dg/cpp0x/explicit14.C: New test.
---
gcc/cp/call.c | 20 +++++++++++++-------
gcc/testsuite/g++.dg/cpp0x/explicit13.C | 14 ++++++++++++++
gcc/testsuite/g++.dg/cpp0x/explicit14.C | 16 ++++++++++++++++
3 files changed, 43 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/explicit13.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/explicit14.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c58231601c9..612ca56cca4 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -92,10 +92,10 @@ struct conversion {
language standards, e.g. disregarding pointer qualifiers or
converting integers to pointers. */
BOOL_BITFIELD bad_p : 1;
- /* If KIND is ck_ref_bind ck_base_conv, true to indicate that a
+ /* If KIND is ck_ref_bind or ck_base, true to indicate that a
temporary should be created to hold the result of the
- conversion. If KIND is ck_ambig or ck_user, true means force
- copy-initialization. */
+ conversion. If KIND is ck_ambig, ck_rvalue, or ck_user, true means
+ force copy-initialization. */
BOOL_BITFIELD need_temporary_p : 1;
/* If KIND is ck_ptr or ck_pmem, true to indicate that a conversion
from a pointer-to-derived to pointer-to-base is being performed. */
@@ -1252,6 +1252,8 @@ standard_conversion (tree to, tree from, tree expr, bool
c_cast_p,
if (flags & LOOKUP_PREFER_RVALUE)
/* Tell convert_like_real to set LOOKUP_PREFER_RVALUE. */
conv->rvaluedness_matches_p = true;
+ if (flags & LOOKUP_ONLYCONVERTING)
+ conv->need_temporary_p = true;
}
/* Allow conversion between `__complex__' data types. */
@@ -7653,12 +7655,16 @@ convert_like_real (conversion *convs, tree expr, tree
fn, int argnum,
type is the same class as, or a derived class of, the class of the
destination [is treated as direct-initialization]. [dcl.init] */
flags = LOOKUP_NORMAL;
+ /* This conversion is being done in the context of a user-defined
+ conversion (i.e. the second step of copy-initialization), so
+ don't allow any more. */
if (convs->user_conv_p)
- /* This conversion is being done in the context of a user-defined
- conversion (i.e. the second step of copy-initialization), so
- don't allow any more. */
flags |= LOOKUP_NO_CONVERSION;
- else
+ /* We might be performing a conversion of the argument
+ to the user-defined conversion, i.e., not a conversion of the
+ result of the user-defined conversion. In which case we skip
+ explicit constructors. */
+ if (convs->need_temporary_p)
flags |= LOOKUP_ONLYCONVERTING;
if (convs->rvaluedness_matches_p)
/* standard_conversion got LOOKUP_PREFER_RVALUE. */
diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit13.C
b/gcc/testsuite/g++.dg/cpp0x/explicit13.C
new file mode 100644
index 00000000000..cbd9a73d8fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/explicit13.C
@@ -0,0 +1,14 @@
+// PR c++/90320
+// { dg-do compile { target c++11 } }
+
+struct M {
+ M() = default;
+ template <typename T> explicit M(T&&) = delete;
+};
+
+struct V {
+ V(M m);
+};
+
+M m;
+V v = m;
diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit14.C
b/gcc/testsuite/g++.dg/cpp0x/explicit14.C
new file mode 100644
index 00000000000..8a8adfe1677
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/explicit14.C
@@ -0,0 +1,16 @@
+// PR c++/90320
+// { dg-do compile { target c++11 } }
+
+struct B { };
+
+struct M : B {
+ M() = default;
+ template <typename T> explicit M(T&&) = delete;
+};
+
+struct V {
+ V(B);
+};
+
+M m;
+V v = m;
base-commit: 8c9d69bafc8fc1f31f6cb50dffab106641d086d0
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
