| Message ID | 20200408215221.2547676-1-polacek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Fix wrong paren-init of aggregates interference [PR93790] | expand |
On 4/8/20 5:52 PM, Marek Polacek wrote: > This PR points out that we are rejecting valid code in C++20. The > problem is that we were surreptitiously transforming > > T& t(e) > > into > > T& t{e} > > which is wrong, because the type of e had a conversion function to T, > while aggregate initialization of t from e doesn't work. Therefore, I > was violating a design principle of P0960, which says that any existing > meaning of A(b) should not change. So I think we should only attempt to > aggregate-initialize if the original expression was ill-formed. > > Another design principle is that () should work where {} works, so this: > > struct S { int i; }; > const S& s(1); > > has to keep working. Thus the special handling for paren-lists with one > element. (A paren-list with more than one element would give you "error: > expression list treated as compound expression in initializer" C++17.) > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > PR c++/93790 > * call.c (initialize_reference): If the reference binding failed, maybe > try initializing from { }. > * decl.c (grok_reference_init): For T& t(e), set > LOOKUP_AGGREGATE_PAREN_INIT but don't build up a constructor yet. > > * g++.dg/cpp2a/paren-init23.C: New test. > * g++.dg/init/aggr14.C: New test. > --- > gcc/cp/call.c | 14 ++++++++++++++ > gcc/cp/decl.c | 19 ++++++++++++++++--- > gcc/testsuite/g++.dg/cpp2a/paren-init23.C | 19 +++++++++++++++++++ > gcc/testsuite/g++.dg/init/aggr14.C | 14 ++++++++++++++ > 4 files changed, 63 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init23.C > create mode 100644 gcc/testsuite/g++.dg/init/aggr14.C > > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 02220ffb3a1..1f3d9d23b5b 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -12196,6 +12196,20 @@ initialize_reference (tree type, tree expr, > > conv = reference_binding (type, TREE_TYPE (expr), expr, /*c_cast_p=*/false, > flags, complain); > + /* If this conversion failed, we're in C++20, and we have something like > + A& a(b) where A is an aggregate, try again, this time as A& a{b}. */ > + if ((!conv || conv->bad_p) > + && (flags & LOOKUP_AGGREGATE_PAREN_INIT)) > + { > + tree e = build_constructor_single (init_list_type_node, NULL_TREE, expr); > + CONSTRUCTOR_IS_DIRECT_INIT (e) = true; > + CONSTRUCTOR_IS_PAREN_INIT (e) = true; > + conversion *c = reference_binding (type, TREE_TYPE (e), e, > + /*c_cast_p=*/false, flags, complain); > + /* If this worked, use it. */ > + if (c && !c->bad_p) > + expr = e, conv = c; > + } > if (!conv || conv->bad_p) > { > if (complain & tf_error) > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > index a6a1340e631..1447b89e692 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -5568,9 +5568,22 @@ grok_reference_init (tree decl, tree type, tree init, int flags) > && !DECL_DECOMPOSITION_P (decl) > && (cxx_dialect >= cxx2a)) > { > - init = build_constructor_from_list (init_list_type_node, init); > - CONSTRUCTOR_IS_DIRECT_INIT (init) = true; > - CONSTRUCTOR_IS_PAREN_INIT (init) = true; > + /* We don't know yet if we should treat const A& r(1) as > + const A& r{1}. */ > + if (list_length (init) == 1) > + { > + flags |= LOOKUP_AGGREGATE_PAREN_INIT; > + init = build_x_compound_expr_from_list (init, ELK_INIT, > + tf_warning_or_error); > + } > + /* If the list had more than one element, the code is ill-formed > + pre-C++20, so we can build a constructor right away. */ > + else > + { > + init = build_constructor_from_list (init_list_type_node, init); > + CONSTRUCTOR_IS_DIRECT_INIT (init) = true; > + CONSTRUCTOR_IS_PAREN_INIT (init) = true; > + } > } > else > init = build_x_compound_expr_from_list (init, ELK_INIT, > diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init23.C b/gcc/testsuite/g++.dg/cpp2a/paren-init23.C > new file mode 100644 > index 00000000000..6038f63be5a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init23.C > @@ -0,0 +1,19 @@ > +// PR c++/93790 - wrong paren-init of aggregates interference. > +// { dg-do compile { target c++2a } } > + > +struct S { > + int i; > +}; > +const S& s(1); > + > +struct A { > + int i; > + A(int); > +}; > +const A& a(1); > + > +struct B { > + int i; > + B(int) = delete; > +}; > +const B& b(1); // { dg-error "use of deleted function" } > diff --git a/gcc/testsuite/g++.dg/init/aggr14.C b/gcc/testsuite/g++.dg/init/aggr14.C > new file mode 100644 > index 00000000000..538b467d467 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/init/aggr14.C > @@ -0,0 +1,14 @@ > +// PR c++/93790 - wrong paren-init of aggregates interference. > +// { dg-do compile } > + > +struct S {}; > +class S_refwrap { > + S& Sref_; > +public: > + S_refwrap(S& Sref) : Sref_(Sref) {} > + operator S&() { return Sref_; } > +}; > + > +S s; > +S_refwrap r(s); > +S& s2(r); > > base-commit: 48242b2c3ac96c0bd5265e77013583ac9ceee97d >
diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 02220ffb3a1..1f3d9d23b5b 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -12196,6 +12196,20 @@ initialize_reference (tree type, tree expr, conv = reference_binding (type, TREE_TYPE (expr), expr, /*c_cast_p=*/false, flags, complain); + /* If this conversion failed, we're in C++20, and we have something like + A& a(b) where A is an aggregate, try again, this time as A& a{b}. */ + if ((!conv || conv->bad_p) + && (flags & LOOKUP_AGGREGATE_PAREN_INIT)) + { + tree e = build_constructor_single (init_list_type_node, NULL_TREE, expr); + CONSTRUCTOR_IS_DIRECT_INIT (e) = true; + CONSTRUCTOR_IS_PAREN_INIT (e) = true; + conversion *c = reference_binding (type, TREE_TYPE (e), e, + /*c_cast_p=*/false, flags, complain); + /* If this worked, use it. */ + if (c && !c->bad_p) + expr = e, conv = c; + } if (!conv || conv->bad_p) { if (complain & tf_error) diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a6a1340e631..1447b89e692 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -5568,9 +5568,22 @@ grok_reference_init (tree decl, tree type, tree init, int flags) && !DECL_DECOMPOSITION_P (decl) && (cxx_dialect >= cxx2a)) { - init = build_constructor_from_list (init_list_type_node, init); - CONSTRUCTOR_IS_DIRECT_INIT (init) = true; - CONSTRUCTOR_IS_PAREN_INIT (init) = true; + /* We don't know yet if we should treat const A& r(1) as + const A& r{1}. */ + if (list_length (init) == 1) + { + flags |= LOOKUP_AGGREGATE_PAREN_INIT; + init = build_x_compound_expr_from_list (init, ELK_INIT, + tf_warning_or_error); + } + /* If the list had more than one element, the code is ill-formed + pre-C++20, so we can build a constructor right away. */ + else + { + init = build_constructor_from_list (init_list_type_node, init); + CONSTRUCTOR_IS_DIRECT_INIT (init) = true; + CONSTRUCTOR_IS_PAREN_INIT (init) = true; + } } else init = build_x_compound_expr_from_list (init, ELK_INIT, diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init23.C b/gcc/testsuite/g++.dg/cpp2a/paren-init23.C new file mode 100644 index 00000000000..6038f63be5a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init23.C @@ -0,0 +1,19 @@ +// PR c++/93790 - wrong paren-init of aggregates interference. +// { dg-do compile { target c++2a } } + +struct S { + int i; +}; +const S& s(1); + +struct A { + int i; + A(int); +}; +const A& a(1); + +struct B { + int i; + B(int) = delete; +}; +const B& b(1); // { dg-error "use of deleted function" } diff --git a/gcc/testsuite/g++.dg/init/aggr14.C b/gcc/testsuite/g++.dg/init/aggr14.C new file mode 100644 index 00000000000..538b467d467 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/aggr14.C @@ -0,0 +1,14 @@ +// PR c++/93790 - wrong paren-init of aggregates interference. +// { dg-do compile } + +struct S {}; +class S_refwrap { + S& Sref_; +public: + S_refwrap(S& Sref) : Sref_(Sref) {} + operator S&() { return Sref_; } +}; + +S s; +S_refwrap r(s); +S& s2(r);
This PR points out that we are rejecting valid code in C++20. The problem is that we were surreptitiously transforming T& t(e) into T& t{e} which is wrong, because the type of e had a conversion function to T, while aggregate initialization of t from e doesn't work. Therefore, I was violating a design principle of P0960, which says that any existing meaning of A(b) should not change. So I think we should only attempt to aggregate-initialize if the original expression was ill-formed. Another design principle is that () should work where {} works, so this: struct S { int i; }; const S& s(1); has to keep working. Thus the special handling for paren-lists with one element. (A paren-list with more than one element would give you "error: expression list treated as compound expression in initializer" C++17.) Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/93790 * call.c (initialize_reference): If the reference binding failed, maybe try initializing from { }. * decl.c (grok_reference_init): For T& t(e), set LOOKUP_AGGREGATE_PAREN_INIT but don't build up a constructor yet. * g++.dg/cpp2a/paren-init23.C: New test. * g++.dg/init/aggr14.C: New test. --- gcc/cp/call.c | 14 ++++++++++++++ gcc/cp/decl.c | 19 ++++++++++++++++--- gcc/testsuite/g++.dg/cpp2a/paren-init23.C | 19 +++++++++++++++++++ gcc/testsuite/g++.dg/init/aggr14.C | 14 ++++++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init23.C create mode 100644 gcc/testsuite/g++.dg/init/aggr14.C base-commit: 48242b2c3ac96c0bd5265e77013583ac9ceee97d