c++: CWG 2359, wrong copy-init with designated init [PR91319]
Checks
Commit Message
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This CWG clarifies that designated initializer support direct-initialization.
Just be careful what Note 2 in [dcl.init.aggr]/4.2 says: "If the
initialization is by designated-initializer-clause, its form determines
whether copy-initialization or direct-initialization is performed." Hence
this patch sets CONSTRUCTOR_IS_DIRECT_INIT only when we are dealing with
".x{}", but not ".x = {}".
PR c++/91319
gcc/cp/ChangeLog:
* parser.cc (cp_parser_initializer_list): Set CONSTRUCTOR_IS_DIRECT_INIT
when the designated initializer is of the .x{} form.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/desig30.C: New test.
---
gcc/cp/parser.cc | 6 ++++++
gcc/testsuite/g++.dg/cpp2a/desig30.C | 22 ++++++++++++++++++++++
2 files changed, 28 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig30.C
base-commit: 54cc21eaf5f3eb7f7a508919a086f6c8bf5c4c17
Comments
On Fri, 25 Aug 2023, Marek Polacek via Gcc-patches wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
LGTM
>
> -- >8 --
>
> This CWG clarifies that designated initializer support direct-initialization.
> Just be careful what Note 2 in [dcl.init.aggr]/4.2 says: "If the
> initialization is by designated-initializer-clause, its form determines
> whether copy-initialization or direct-initialization is performed." Hence
> this patch sets CONSTRUCTOR_IS_DIRECT_INIT only when we are dealing with
> ".x{}", but not ".x = {}".
>
> PR c++/91319
>
> gcc/cp/ChangeLog:
>
> * parser.cc (cp_parser_initializer_list): Set CONSTRUCTOR_IS_DIRECT_INIT
> when the designated initializer is of the .x{} form.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp2a/desig30.C: New test.
> ---
> gcc/cp/parser.cc | 6 ++++++
> gcc/testsuite/g++.dg/cpp2a/desig30.C | 22 ++++++++++++++++++++++
> 2 files changed, 28 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig30.C
>
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index eeb22e44fb4..b3d5c65b469 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -25718,6 +25718,7 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> tree designator;
> tree initializer;
> bool clause_non_constant_p;
> + bool direct_p = false;
> location_t loc = cp_lexer_peek_token (parser->lexer)->location;
>
> /* Handle the C++20 syntax, '. id ='. */
> @@ -25740,6 +25741,8 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
> /* Consume the `='. */
> cp_lexer_consume_token (parser->lexer);
> + else
> + direct_p = true;
> }
> /* Also, if the next token is an identifier and the following one is a
> colon, we are looking at the GNU designated-initializer
> @@ -25817,6 +25820,9 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> if (clause_non_constant_p && non_constant_p)
> *non_constant_p = true;
>
> + if (TREE_CODE (initializer) == CONSTRUCTOR)
> + CONSTRUCTOR_IS_DIRECT_INIT (initializer) |= direct_p;
> +
> /* If we have an ellipsis, this is an initializer pack
> expansion. */
> if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
> diff --git a/gcc/testsuite/g++.dg/cpp2a/desig30.C b/gcc/testsuite/g++.dg/cpp2a/desig30.C
> new file mode 100644
> index 00000000000..d9292df9de2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/desig30.C
> @@ -0,0 +1,22 @@
> +// PR c++/91319
> +// { dg-do compile { target c++20 } }
> +
> +struct X {
> + explicit X() { }
> +};
> +
> +struct Aggr {
> + X x;
> +};
> +
> +Aggr
> +f ()
> +{
> + return Aggr{.x{}};
> +}
> +
> +Aggr
> +f2 ()
> +{
> + return Aggr{.x = {}}; // { dg-error "explicit constructor" }
> +}
>
> base-commit: 54cc21eaf5f3eb7f7a508919a086f6c8bf5c4c17
> --
> 2.41.0
>
>
On 8/25/23 12:44, Marek Polacek wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >8 --
>
> This CWG clarifies that designated initializer support direct-initialization.
> Just be careful what Note 2 in [dcl.init.aggr]/4.2 says: "If the
> initialization is by designated-initializer-clause, its form determines
> whether copy-initialization or direct-initialization is performed." Hence
> this patch sets CONSTRUCTOR_IS_DIRECT_INIT only when we are dealing with
> ".x{}", but not ".x = {}".
>
> PR c++/91319
>
> gcc/cp/ChangeLog:
>
> * parser.cc (cp_parser_initializer_list): Set CONSTRUCTOR_IS_DIRECT_INIT
> when the designated initializer is of the .x{} form.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp2a/desig30.C: New test.
> ---
> gcc/cp/parser.cc | 6 ++++++
> gcc/testsuite/g++.dg/cpp2a/desig30.C | 22 ++++++++++++++++++++++
> 2 files changed, 28 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig30.C
>
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index eeb22e44fb4..b3d5c65b469 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -25718,6 +25718,7 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> tree designator;
> tree initializer;
> bool clause_non_constant_p;
> + bool direct_p = false;
> location_t loc = cp_lexer_peek_token (parser->lexer)->location;
>
> /* Handle the C++20 syntax, '. id ='. */
> @@ -25740,6 +25741,8 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
> /* Consume the `='. */
> cp_lexer_consume_token (parser->lexer);
> + else
> + direct_p = true;
> }
> /* Also, if the next token is an identifier and the following one is a
> colon, we are looking at the GNU designated-initializer
> @@ -25817,6 +25820,9 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> if (clause_non_constant_p && non_constant_p)
> *non_constant_p = true;
>
> + if (TREE_CODE (initializer) == CONSTRUCTOR)
> + CONSTRUCTOR_IS_DIRECT_INIT (initializer) |= direct_p;
Why |= rather than = ?
Jason
On Mon, Aug 28, 2023 at 06:27:26PM -0400, Jason Merrill wrote:
> On 8/25/23 12:44, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > -- >8 --
> >
> > This CWG clarifies that designated initializer support direct-initialization.
> > Just be careful what Note 2 in [dcl.init.aggr]/4.2 says: "If the
> > initialization is by designated-initializer-clause, its form determines
> > whether copy-initialization or direct-initialization is performed." Hence
> > this patch sets CONSTRUCTOR_IS_DIRECT_INIT only when we are dealing with
> > ".x{}", but not ".x = {}".
> >
> > PR c++/91319
> >
> > gcc/cp/ChangeLog:
> >
> > * parser.cc (cp_parser_initializer_list): Set CONSTRUCTOR_IS_DIRECT_INIT
> > when the designated initializer is of the .x{} form.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp2a/desig30.C: New test.
> > ---
> > gcc/cp/parser.cc | 6 ++++++
> > gcc/testsuite/g++.dg/cpp2a/desig30.C | 22 ++++++++++++++++++++++
> > 2 files changed, 28 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig30.C
> >
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index eeb22e44fb4..b3d5c65b469 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -25718,6 +25718,7 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> > tree designator;
> > tree initializer;
> > bool clause_non_constant_p;
> > + bool direct_p = false;
> > location_t loc = cp_lexer_peek_token (parser->lexer)->location;
> > /* Handle the C++20 syntax, '. id ='. */
> > @@ -25740,6 +25741,8 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> > if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
> > /* Consume the `='. */
> > cp_lexer_consume_token (parser->lexer);
> > + else
> > + direct_p = true;
> > }
> > /* Also, if the next token is an identifier and the following one is a
> > colon, we are looking at the GNU designated-initializer
> > @@ -25817,6 +25820,9 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> > if (clause_non_constant_p && non_constant_p)
> > *non_constant_p = true;
> > + if (TREE_CODE (initializer) == CONSTRUCTOR)
> > + CONSTRUCTOR_IS_DIRECT_INIT (initializer) |= direct_p;
>
> Why |= rather than = ?
CONSTRUCTOR_IS_DIRECT_INIT could already have been set earlier so using
= might wrongly clear it. I saw this in direct-enum-init1.C.
Marek
On 8/28/23 19:09, Marek Polacek wrote:
> On Mon, Aug 28, 2023 at 06:27:26PM -0400, Jason Merrill wrote:
>> On 8/25/23 12:44, Marek Polacek wrote:
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> -- >8 --
>>>
>>> This CWG clarifies that designated initializer support direct-initialization.
>>> Just be careful what Note 2 in [dcl.init.aggr]/4.2 says: "If the
>>> initialization is by designated-initializer-clause, its form determines
>>> whether copy-initialization or direct-initialization is performed." Hence
>>> this patch sets CONSTRUCTOR_IS_DIRECT_INIT only when we are dealing with
>>> ".x{}", but not ".x = {}".
>>>
>>> PR c++/91319
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * parser.cc (cp_parser_initializer_list): Set CONSTRUCTOR_IS_DIRECT_INIT
>>> when the designated initializer is of the .x{} form.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * g++.dg/cpp2a/desig30.C: New test.
>>> ---
>>> gcc/cp/parser.cc | 6 ++++++
>>> gcc/testsuite/g++.dg/cpp2a/desig30.C | 22 ++++++++++++++++++++++
>>> 2 files changed, 28 insertions(+)
>>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig30.C
>>>
>>> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
>>> index eeb22e44fb4..b3d5c65b469 100644
>>> --- a/gcc/cp/parser.cc
>>> +++ b/gcc/cp/parser.cc
>>> @@ -25718,6 +25718,7 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
>>> tree designator;
>>> tree initializer;
>>> bool clause_non_constant_p;
>>> + bool direct_p = false;
>>> location_t loc = cp_lexer_peek_token (parser->lexer)->location;
>>> /* Handle the C++20 syntax, '. id ='. */
>>> @@ -25740,6 +25741,8 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
>>> if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
>>> /* Consume the `='. */
>>> cp_lexer_consume_token (parser->lexer);
>>> + else
>>> + direct_p = true;
>>> }
>>> /* Also, if the next token is an identifier and the following one is a
>>> colon, we are looking at the GNU designated-initializer
>>> @@ -25817,6 +25820,9 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
>>> if (clause_non_constant_p && non_constant_p)
>>> *non_constant_p = true;
>>> + if (TREE_CODE (initializer) == CONSTRUCTOR)
>>> + CONSTRUCTOR_IS_DIRECT_INIT (initializer) |= direct_p;
>>
>> Why |= rather than = ?
>
> CONSTRUCTOR_IS_DIRECT_INIT could already have been set earlier so using
> = might wrongly clear it. I saw this in direct-enum-init1.C.
What is setting it earlier?
The patch is OK with a comment explaining that.
Jason
On Tue, Aug 29, 2023 at 04:44:11PM -0400, Jason Merrill wrote:
> On 8/28/23 19:09, Marek Polacek wrote:
> > On Mon, Aug 28, 2023 at 06:27:26PM -0400, Jason Merrill wrote:
> > > On 8/25/23 12:44, Marek Polacek wrote:
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > >
> > > > -- >8 --
> > > >
> > > > This CWG clarifies that designated initializer support direct-initialization.
> > > > Just be careful what Note 2 in [dcl.init.aggr]/4.2 says: "If the
> > > > initialization is by designated-initializer-clause, its form determines
> > > > whether copy-initialization or direct-initialization is performed." Hence
> > > > this patch sets CONSTRUCTOR_IS_DIRECT_INIT only when we are dealing with
> > > > ".x{}", but not ".x = {}".
> > > >
> > > > PR c++/91319
> > > >
> > > > gcc/cp/ChangeLog:
> > > >
> > > > * parser.cc (cp_parser_initializer_list): Set CONSTRUCTOR_IS_DIRECT_INIT
> > > > when the designated initializer is of the .x{} form.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > * g++.dg/cpp2a/desig30.C: New test.
> > > > ---
> > > > gcc/cp/parser.cc | 6 ++++++
> > > > gcc/testsuite/g++.dg/cpp2a/desig30.C | 22 ++++++++++++++++++++++
> > > > 2 files changed, 28 insertions(+)
> > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig30.C
> > > >
> > > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > > > index eeb22e44fb4..b3d5c65b469 100644
> > > > --- a/gcc/cp/parser.cc
> > > > +++ b/gcc/cp/parser.cc
> > > > @@ -25718,6 +25718,7 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> > > > tree designator;
> > > > tree initializer;
> > > > bool clause_non_constant_p;
> > > > + bool direct_p = false;
> > > > location_t loc = cp_lexer_peek_token (parser->lexer)->location;
> > > > /* Handle the C++20 syntax, '. id ='. */
> > > > @@ -25740,6 +25741,8 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> > > > if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
> > > > /* Consume the `='. */
> > > > cp_lexer_consume_token (parser->lexer);
> > > > + else
> > > > + direct_p = true;
> > > > }
> > > > /* Also, if the next token is an identifier and the following one is a
> > > > colon, we are looking at the GNU designated-initializer
> > > > @@ -25817,6 +25820,9 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
> > > > if (clause_non_constant_p && non_constant_p)
> > > > *non_constant_p = true;
> > > > + if (TREE_CODE (initializer) == CONSTRUCTOR)
> > > > + CONSTRUCTOR_IS_DIRECT_INIT (initializer) |= direct_p;
> > >
> > > Why |= rather than = ?
> >
> > CONSTRUCTOR_IS_DIRECT_INIT could already have been set earlier so using
> > = might wrongly clear it. I saw this in direct-enum-init1.C.
>
> What is setting it earlier?
cp_parser_functional_cast. Test:
enum class C {};
template <int>
void
foo ()
{
C c = { C{8} };
}
void
test ()
{
foo<0> ();
}
The template actually matters here because then finish_compound_literal
returns {8} and not just 8 due to:
/* If we're in a template, return the original compound literal. */
if (orig_cl)
return orig_cl;
> The patch is OK with a comment explaining that.
Ok, I'll say that CONSTRUCTOR_IS_DIRECT_INIT could have been set in
cp_parser_functional_cast so we must be careful not to clear the flag.
Thanks,
Marek
@@ -25718,6 +25718,7 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
tree designator;
tree initializer;
bool clause_non_constant_p;
+ bool direct_p = false;
location_t loc = cp_lexer_peek_token (parser->lexer)->location;
/* Handle the C++20 syntax, '. id ='. */
@@ -25740,6 +25741,8 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
/* Consume the `='. */
cp_lexer_consume_token (parser->lexer);
+ else
+ direct_p = true;
}
/* Also, if the next token is an identifier and the following one is a
colon, we are looking at the GNU designated-initializer
@@ -25817,6 +25820,9 @@ cp_parser_initializer_list (cp_parser* parser, bool* non_constant_p,
if (clause_non_constant_p && non_constant_p)
*non_constant_p = true;
+ if (TREE_CODE (initializer) == CONSTRUCTOR)
+ CONSTRUCTOR_IS_DIRECT_INIT (initializer) |= direct_p;
+
/* If we have an ellipsis, this is an initializer pack
expansion. */
if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
new file mode 100644
@@ -0,0 +1,22 @@
+// PR c++/91319
+// { dg-do compile { target c++20 } }
+
+struct X {
+ explicit X() { }
+};
+
+struct Aggr {
+ X x;
+};
+
+Aggr
+f ()
+{
+ return Aggr{.x{}};
+}
+
+Aggr
+f2 ()
+{
+ return Aggr{.x = {}}; // { dg-error "explicit constructor" }
+}