| Message ID | 71bbf73e-f7c5-3d0d-cc33-ef2921ea8e55@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] PR 82235 (Copy ctor is not found for copying array of an object when it's marked explicit) | expand |
Hi, On 15/11/2017 00:54, Mukesh Kapoor wrote: > Hi, > > This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82235 > For the following test case > > struct Foo { > Foo() {} > explicit Foo(const Foo& aOther) {} > }; > struct Bar { > Foo m[1]; > }; > void test() { > Bar a; > Bar b = a; > } > > the compiler issues an error when the compiler generated copy > constructor of class Bar calls the explicit copy constructor of class > Foo. The fix is to implement ISO C++/17 16.3.1.4 (over.match.copy) > correctly. I'm pinging this patch sent a while by Mukesh (I'm taking over from him about it). Any comments? https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01133.html Thanks! Paolo.
On 12/12/2017 03:20 PM, Paolo Carlini wrote: > Hi, > > On 15/11/2017 00:54, Mukesh Kapoor wrote: >> Hi, >> >> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82235 >> For the following test case >> >> struct Foo { >> Foo() {} >> explicit Foo(const Foo& aOther) {} >> }; >> struct Bar { >> Foo m[1]; >> }; >> void test() { >> Bar a; >> Bar b = a; >> } >> >> the compiler issues an error when the compiler generated copy >> constructor of class Bar calls the explicit copy constructor of class >> Foo. The fix is to implement ISO C++/17 16.3.1.4 (over.match.copy) >> correctly. > I'm pinging this patch sent a while by Mukesh (I'm taking over from him > about it). Any comments? > > https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01133.html These two don't match: > + When initializing a temporary to be bound to the first > + parameter of a constructor where the parameter is of type > +/* Return true if current_function_decl is a constructor > + and its first argument is a reference type and it is The language is talking about the function being called, and ref_first_parm_of_constructor is looking at the function we're currently in. Jason
Hi Jason, On 13/12/2017 23:27, Jason Merrill wrote: > These two don't match: > >> + When initializing a temporary to be bound to the first >> + parameter of a constructor where the parameter is of type > >> +/* Return true if current_function_decl is a constructor >> + and its first argument is a reference type and it is > > The language is talking about the function being called, and > ref_first_parm_of_constructor is looking at the function we're > currently in. Indeed. Thanks Jason for the feedback. I was going to send an improved patch, among other things using CP_DECL_CONTEXT, when I noticed that this issue seems essentially a special case of *your* core/899, right? For 899 we already have some infrastructure in place and I believe we only have to figure out why we don't handle correctly *arrays*, because otherwise we already accept a similar testcase involving a plain 'Foo m' member. Please let me know, otherwise I'm going to work in this direction! Thanks again, Paolo.
Index: gcc/cp/init.c =================================================================== --- gcc/cp/init.c (revision 254412) +++ gcc/cp/init.c (working copy) @@ -1615,6 +1615,26 @@ expand_member_init (tree name) return NULL_TREE; } +/* Return true if current_function_decl is a constructor + and its first argument is a reference type and it is + a direct initialization. + If FLAGS is 0 then it is a direct initialization, the (init) form. */ + +static inline bool +ref_first_parm_of_constructor (int flags) +{ + bool res = false; + if (current_function_decl != NULL_TREE) + { + tree parmlist = FUNCTION_FIRST_USER_PARMTYPE (current_function_decl); + tree firstparm = TREE_VALUE (parmlist); + res = ((flags == 0) && DECL_CONSTRUCTOR_P (current_function_decl) + && (TREE_CODE (firstparm) == REFERENCE_TYPE)); + } + + return res; +} + /* This is like `expand_member_init', only it stores one aggregate value into another. @@ -1728,10 +1748,20 @@ build_aggr_init (tree exp, tree init, int flags, t return stmt_expr; } + /* Don't set LOOKUP_ONLYCONVERTING for the following special case: + ISO C++/17 16.3.1.4 (over.match.copy): + When initializing a temporary to be bound to the first + parameter of a constructor where the parameter is of type + "reference to possibly cv-qualified T" and the constructor + is called with a single argument in the context of + direct-initialization of an object of type "cv2 T", explicit + conversion functions are also considered. */ + if (init && init != void_type_node && TREE_CODE (init) != TREE_LIST && !(TREE_CODE (init) == TARGET_EXPR && TARGET_EXPR_DIRECT_INIT_P (init)) + && !ref_first_parm_of_constructor (flags) && !DIRECT_LIST_INIT_P (init)) flags |= LOOKUP_ONLYCONVERTING; Index: gcc/testsuite/g++.dg/cpp0x/explicit13.C =================================================================== --- gcc/testsuite/g++.dg/cpp0x/explicit13.C (revision 0) +++ gcc/testsuite/g++.dg/cpp0x/explicit13.C (working copy) @@ -0,0 +1,17 @@ +// PR c++/82235 +// { dg-do compile { target c++11 } } + +struct Foo { + Foo() {} + explicit Foo(const Foo& aOther) {} +}; + +struct Bar { + Foo m[1]; +}; + +void test() +{ + Bar a; + Bar b = a; +}
Hi, This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82235 For the following test case struct Foo { Foo() {} explicit Foo(const Foo& aOther) {} }; struct Bar { Foo m[1]; }; void test() { Bar a; Bar b = a; } the compiler issues an error when the compiler generated copy constructor of class Bar calls the explicit copy constructor of class Foo. The fix is to implement ISO C++/17 16.3.1.4 (over.match.copy) correctly. Bootstrapped and tested with 'make check' on x86_64-linux. Mukesh /gcc/cp 2017-11-14 Mukesh Kapoor <mukesh.kapoor@oracle.com> PR c++/82235 * init.c (build_aggr_init): Allow explicit constructors for the following special case: ISO C++/17 16.3.1.4 (over.match.copy): When initializing a temporary to be bound to the first parameter of a constructor where the parameter is of type "reference to possibly cv-qualified T" and the constructor is called with a single argument in the context of direct-initialization of an object of type "cv2 T", explicit conversion functions are also considered. Added function ref_first_parm_of_constructor() to check for this case. In build_aggr_init(), don't set LOOKUP_ONLYCONVERTING in flags if ref_first_parm_of_constructor() returns true. /testsuite 2017-11-14 Mukesh Kapoor <mukesh.kapoor@oracle.com> PR c++/82235 * g++.dg/cpp0x/explicit13.C: New