Re: [PATCH] c++: Explicit constructor called in copy-initialization [PR90320]

2020-04-23 Thread Jason Merrill via Gcc-patches

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&) 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) : 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?


@@ -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;
+   }

[PATCH] c++: Explicit constructor called in copy-initialization [PR90320]

2020-04-22 Thread Marek Polacek via Gcc-patches
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&) 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) : 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;
 }
 
/* Allow conversion between `__complex__' data types.  */
@@ -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;