| Message ID | 20220321204614.2409000-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [pushed] c++: designated init and aggregate members [PR102337] | expand |
On Mon, 21 Mar 2022, Jason Merrill via Gcc-patches wrote: > Our C++20 designated initializer handling was broken with members of class > type; we would find the relevant member and then try to find a member of > the member with the same name. Or we would sometimes ignore the designator > entirely. The former problem is fixed by the change to reshape_init_class, > the latter by the change to reshape_init_r. > > Tested x86_64-pc-linux-gnu, applying to trunk. > > PR c++/103337 > PR c++/102740 > PR c++/103299 > PR c++/102538 > > gcc/cp/ChangeLog: > > * decl.cc (reshape_init_class): Avoid looking for designator > after we found it. > (reshape_init_r): Keep looking for designator. > > gcc/testsuite/ChangeLog: > > * g++.dg/ext/flexary3.C: Remove one error. > * g++.dg/parse/pr43765.C: Likewise. > * g++.dg/cpp2a/desig22.C: New test. > * g++.dg/cpp2a/desig23.C: New test. > * g++.dg/cpp2a/desig24.C: New test. > * g++.dg/cpp2a/desig25.C: New test. > --- > gcc/cp/decl.cc | 47 +++++++++++++++++++++++++--- > gcc/testsuite/g++.dg/cpp2a/desig22.C | 11 +++++++ > gcc/testsuite/g++.dg/cpp2a/desig23.C | 20 ++++++++++++ > gcc/testsuite/g++.dg/cpp2a/desig24.C | 11 +++++++ > gcc/testsuite/g++.dg/cpp2a/desig25.C | 13 ++++++++ > gcc/testsuite/g++.dg/ext/flexary3.C | 2 +- > gcc/testsuite/g++.dg/parse/pr43765.C | 6 ++-- > 7 files changed, 101 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig22.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig23.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig24.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig25.C > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 375385e0013..34d9dad9fb0 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -6598,8 +6598,9 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, > { > tree field_init; > constructor_elt *old_cur = d->cur; > + bool direct_desig = false; > > - /* Handle designated initializers, as an extension. */ > + /* Handle C++20 designated initializers. */ > if (d->cur->index) > { > if (d->cur->index == error_mark_node) > @@ -6617,7 +6618,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, > } > } > else if (TREE_CODE (d->cur->index) == IDENTIFIER_NODE) > - field = get_class_binding (type, d->cur->index); > + { > + field = get_class_binding (type, d->cur->index); > + direct_desig = true; > + } > else > { > if (complain & tf_error) > @@ -6669,6 +6673,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, > break; > gcc_assert (aafield); > field = aafield; > + direct_desig = false; > } > } > > @@ -6683,9 +6688,32 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, > assumed to correspond to no elements of the initializer list. */ > goto continue_; > > - field_init = reshape_init_r (TREE_TYPE (field), d, > - /*first_initializer_p=*/NULL_TREE, > - complain); > + if (direct_desig) > + { > + /* The designated field F is initialized from this one element: > + Temporarily clear the designator so a recursive reshape_init_class > + doesn't try to find it again in F, and adjust d->end so we don't > + try to use the next initializer to initialize another member of F. > + > + Note that we don't want these changes if we found the designator > + inside an anon aggr above; we leave them alone to implement: > + > + "If the element is an anonymous union member and the initializer > + list is a brace-enclosed designated- initializer-list, the element > + is initialized by the designated-initializer-list { D }, where D > + is the designated- initializer-clause naming a member of the > + anonymous union member." */ > + auto end_ = make_temp_override (d->end, d->cur + 1); > + auto idx_ = make_temp_override (d->cur->index, NULL_TREE); IIUC this override of d->cur->index mutates the underlying CONSTRUCTOR, which is probably fine, but I wonder if it'd be worthwhile to avoid the mutation by using a temporary reshape_iter for the recursive call, something like: constructor_elt elt = { /*index=*/NULL_TREE, d->cur->value }; reshape_iter iter = { &elt, &elt + 1 }; field_init = reshape_init_r (TREE_TYPE (field), &iter, /*first_initializer_p=*/NULL_TREE, complain); d->cur++; > + field_init = reshape_init_r (TREE_TYPE (field), d, > + /*first_initializer_p=*/NULL_TREE, > + complain); > + } > + else > + field_init = reshape_init_r (TREE_TYPE (field), d, > + /*first_initializer_p=*/NULL_TREE, > + complain); > + > if (field_init == error_mark_node) > return error_mark_node; > > @@ -6941,6 +6969,15 @@ reshape_init_r (tree type, reshape_iter *d, tree first_initializer_p, > to handle initialization of arrays and similar. */ > else if (COMPOUND_LITERAL_P (stripped_init)) > gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init)); > + /* If we have an unresolved designator, we need to find the member it > + designates within TYPE, so proceed to the routines below. For > + FIELD_DECL or INTEGER_CST designators, we're already initializing > + the designated element. */ > + else if (d->cur->index > + && TREE_CODE (d->cur->index) == IDENTIFIER_NODE) > + /* Brace elision with designators is only permitted for anonymous > + aggregates. */ > + gcc_checking_assert (ANON_AGGR_TYPE_P (type)); > /* A CONSTRUCTOR of the target's type is a previously > digested initializer. */ > else if (same_type_ignoring_top_level_qualifiers_p (type, init_type)) > diff --git a/gcc/testsuite/g++.dg/cpp2a/desig22.C b/gcc/testsuite/g++.dg/cpp2a/desig22.C > new file mode 100644 > index 00000000000..ba083f8e3d5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/desig22.C > @@ -0,0 +1,11 @@ > +// PR c++/103337 > +// { dg-do compile { target c++20 } } > + > +struct op_t { > + struct put_t { > + int x; > + } put; > +}; > + > +op_t x{0}; // OK > +op_t y{.put=0}; // bogus error: 'op_t::put_t' has no non-static data member named 'put' > diff --git a/gcc/testsuite/g++.dg/cpp2a/desig23.C b/gcc/testsuite/g++.dg/cpp2a/desig23.C > new file mode 100644 > index 00000000000..4354e644f6a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/desig23.C > @@ -0,0 +1,20 @@ > +// PR c++/102740 > +// { dg-do compile { target c++20 } } > +// { dg-additional-options -Wmissing-braces } > + > +typedef struct { > + union { > + struct { > + const void* content; > + } put; > + }; > +} op_t; > + > +op_t f(const char* alias) { > + return op_t{ > + .put = > + { > + .content = alias, > + }, > + }; // { dg-warning "missing braces" } > +} > diff --git a/gcc/testsuite/g++.dg/cpp2a/desig24.C b/gcc/testsuite/g++.dg/cpp2a/desig24.C > new file mode 100644 > index 00000000000..219cc9c3b8e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/desig24.C > @@ -0,0 +1,11 @@ > +// PR c++/103299 > +// { dg-do compile { target c++20 } } > + > +struct foo { > + union { > + int fp1{}; > + char fp2; > + }; > +}; > + > +static_assert(foo{.fp2={}}.fp2 == 0); > diff --git a/gcc/testsuite/g++.dg/cpp2a/desig25.C b/gcc/testsuite/g++.dg/cpp2a/desig25.C > new file mode 100644 > index 00000000000..9da958c29e9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/desig25.C > @@ -0,0 +1,13 @@ > +// PR c++/102538 > +// { dg-do run { target c++20 } } > + > +struct X { union { char r8[8]; int r32[2]; }; }; > +struct Y { X v[1]; }; > +Y x = { { { .r32 = { 5, 6 } } } }; > + > +int > +main () > +{ > + if (x.v[0].r32[0] != 5 || x.v[0].r32[1] != 6) > + __builtin_abort (); > +} > diff --git a/gcc/testsuite/g++.dg/ext/flexary3.C b/gcc/testsuite/g++.dg/ext/flexary3.C > index 34b17254f8c..8344b42dd16 100644 > --- a/gcc/testsuite/g++.dg/ext/flexary3.C > +++ b/gcc/testsuite/g++.dg/ext/flexary3.C > @@ -16,7 +16,7 @@ struct s { > > int main() > { > - struct s s = { .c = 0 }; // { dg-error "initializer" } > + struct s s = { .c = 0 }; > // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } > return 0; > } > diff --git a/gcc/testsuite/g++.dg/parse/pr43765.C b/gcc/testsuite/g++.dg/parse/pr43765.C > index 5e602204007..aa099a4d20b 100644 > --- a/gcc/testsuite/g++.dg/parse/pr43765.C > +++ b/gcc/testsuite/g++.dg/parse/pr43765.C > @@ -12,6 +12,6 @@ SomeType vals[] = > { > { 0, values : temp, }, // { dg-error "either all initializer clauses should be designated or none of them should be" "" { target c++2a } } > 0 > - }; // { dg-error "GNU-style designated initializer for an array|cannot convert" } > -// (note the error above is on the wrong line) > - // { dg-error "initialization of flexible array member in a nested context" "" { target *-*-* } .-2 } > + }; > +// (note the error below is on the wrong line) > +// { dg-error "initialization of flexible array member in a nested context" "" { target *-*-* } .-2 } > > base-commit: 70b8f43695b0e6fabc760d247ac83f354092b21d > prerequisite-patch-id: d89344aafac30ce0b8645945beb2d94b7ddb3515 > -- > 2.27.0 > >
On 3/22/22 10:19, Patrick Palka wrote: > On Mon, 21 Mar 2022, Jason Merrill via Gcc-patches wrote: > >> Our C++20 designated initializer handling was broken with members of class >> type; we would find the relevant member and then try to find a member of >> the member with the same name. Or we would sometimes ignore the designator >> entirely. The former problem is fixed by the change to reshape_init_class, >> the latter by the change to reshape_init_r. >> >> Tested x86_64-pc-linux-gnu, applying to trunk. >> >> PR c++/103337 >> PR c++/102740 >> PR c++/103299 >> PR c++/102538 >> >> gcc/cp/ChangeLog: >> >> * decl.cc (reshape_init_class): Avoid looking for designator >> after we found it. >> (reshape_init_r): Keep looking for designator. >> >> gcc/testsuite/ChangeLog: >> >> * g++.dg/ext/flexary3.C: Remove one error. >> * g++.dg/parse/pr43765.C: Likewise. >> * g++.dg/cpp2a/desig22.C: New test. >> * g++.dg/cpp2a/desig23.C: New test. >> * g++.dg/cpp2a/desig24.C: New test. >> * g++.dg/cpp2a/desig25.C: New test. >> --- >> gcc/cp/decl.cc | 47 +++++++++++++++++++++++++--- >> gcc/testsuite/g++.dg/cpp2a/desig22.C | 11 +++++++ >> gcc/testsuite/g++.dg/cpp2a/desig23.C | 20 ++++++++++++ >> gcc/testsuite/g++.dg/cpp2a/desig24.C | 11 +++++++ >> gcc/testsuite/g++.dg/cpp2a/desig25.C | 13 ++++++++ >> gcc/testsuite/g++.dg/ext/flexary3.C | 2 +- >> gcc/testsuite/g++.dg/parse/pr43765.C | 6 ++-- >> 7 files changed, 101 insertions(+), 9 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig22.C >> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig23.C >> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig24.C >> create mode 100644 gcc/testsuite/g++.dg/cpp2a/desig25.C >> >> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc >> index 375385e0013..34d9dad9fb0 100644 >> --- a/gcc/cp/decl.cc >> +++ b/gcc/cp/decl.cc >> @@ -6598,8 +6598,9 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, >> { >> tree field_init; >> constructor_elt *old_cur = d->cur; >> + bool direct_desig = false; >> >> - /* Handle designated initializers, as an extension. */ >> + /* Handle C++20 designated initializers. */ >> if (d->cur->index) >> { >> if (d->cur->index == error_mark_node) >> @@ -6617,7 +6618,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, >> } >> } >> else if (TREE_CODE (d->cur->index) == IDENTIFIER_NODE) >> - field = get_class_binding (type, d->cur->index); >> + { >> + field = get_class_binding (type, d->cur->index); >> + direct_desig = true; >> + } >> else >> { >> if (complain & tf_error) >> @@ -6669,6 +6673,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, >> break; >> gcc_assert (aafield); >> field = aafield; >> + direct_desig = false; >> } >> } >> >> @@ -6683,9 +6688,32 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, >> assumed to correspond to no elements of the initializer list. */ >> goto continue_; >> >> - field_init = reshape_init_r (TREE_TYPE (field), d, >> - /*first_initializer_p=*/NULL_TREE, >> - complain); >> + if (direct_desig) >> + { >> + /* The designated field F is initialized from this one element: >> + Temporarily clear the designator so a recursive reshape_init_class >> + doesn't try to find it again in F, and adjust d->end so we don't >> + try to use the next initializer to initialize another member of F. >> + >> + Note that we don't want these changes if we found the designator >> + inside an anon aggr above; we leave them alone to implement: >> + >> + "If the element is an anonymous union member and the initializer >> + list is a brace-enclosed designated- initializer-list, the element >> + is initialized by the designated-initializer-list { D }, where D >> + is the designated- initializer-clause naming a member of the >> + anonymous union member." */ >> + auto end_ = make_temp_override (d->end, d->cur + 1); >> + auto idx_ = make_temp_override (d->cur->index, NULL_TREE); > > IIUC this override of d->cur->index mutates the underlying CONSTRUCTOR, which > is probably fine, but I wonder if it'd be worthwhile to avoid the mutation by > using a temporary reshape_iter for the recursive call, something like: > > constructor_elt elt = { /*index=*/NULL_TREE, d->cur->value }; > reshape_iter iter = { &elt, &elt + 1 }; > field_init = reshape_init_r (TREE_TYPE (field), &iter, > /*first_initializer_p=*/NULL_TREE, > complain); > d->cur++; Sure, makes sense. I also decided to factor it out.
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 375385e0013..34d9dad9fb0 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -6598,8 +6598,9 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, { tree field_init; constructor_elt *old_cur = d->cur; + bool direct_desig = false; - /* Handle designated initializers, as an extension. */ + /* Handle C++20 designated initializers. */ if (d->cur->index) { if (d->cur->index == error_mark_node) @@ -6617,7 +6618,10 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, } } else if (TREE_CODE (d->cur->index) == IDENTIFIER_NODE) - field = get_class_binding (type, d->cur->index); + { + field = get_class_binding (type, d->cur->index); + direct_desig = true; + } else { if (complain & tf_error) @@ -6669,6 +6673,7 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, break; gcc_assert (aafield); field = aafield; + direct_desig = false; } } @@ -6683,9 +6688,32 @@ reshape_init_class (tree type, reshape_iter *d, bool first_initializer_p, assumed to correspond to no elements of the initializer list. */ goto continue_; - field_init = reshape_init_r (TREE_TYPE (field), d, - /*first_initializer_p=*/NULL_TREE, - complain); + if (direct_desig) + { + /* The designated field F is initialized from this one element: + Temporarily clear the designator so a recursive reshape_init_class + doesn't try to find it again in F, and adjust d->end so we don't + try to use the next initializer to initialize another member of F. + + Note that we don't want these changes if we found the designator + inside an anon aggr above; we leave them alone to implement: + + "If the element is an anonymous union member and the initializer + list is a brace-enclosed designated- initializer-list, the element + is initialized by the designated-initializer-list { D }, where D + is the designated- initializer-clause naming a member of the + anonymous union member." */ + auto end_ = make_temp_override (d->end, d->cur + 1); + auto idx_ = make_temp_override (d->cur->index, NULL_TREE); + field_init = reshape_init_r (TREE_TYPE (field), d, + /*first_initializer_p=*/NULL_TREE, + complain); + } + else + field_init = reshape_init_r (TREE_TYPE (field), d, + /*first_initializer_p=*/NULL_TREE, + complain); + if (field_init == error_mark_node) return error_mark_node; @@ -6941,6 +6969,15 @@ reshape_init_r (tree type, reshape_iter *d, tree first_initializer_p, to handle initialization of arrays and similar. */ else if (COMPOUND_LITERAL_P (stripped_init)) gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init)); + /* If we have an unresolved designator, we need to find the member it + designates within TYPE, so proceed to the routines below. For + FIELD_DECL or INTEGER_CST designators, we're already initializing + the designated element. */ + else if (d->cur->index + && TREE_CODE (d->cur->index) == IDENTIFIER_NODE) + /* Brace elision with designators is only permitted for anonymous + aggregates. */ + gcc_checking_assert (ANON_AGGR_TYPE_P (type)); /* A CONSTRUCTOR of the target's type is a previously digested initializer. */ else if (same_type_ignoring_top_level_qualifiers_p (type, init_type)) diff --git a/gcc/testsuite/g++.dg/cpp2a/desig22.C b/gcc/testsuite/g++.dg/cpp2a/desig22.C new file mode 100644 index 00000000000..ba083f8e3d5 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/desig22.C @@ -0,0 +1,11 @@ +// PR c++/103337 +// { dg-do compile { target c++20 } } + +struct op_t { + struct put_t { + int x; + } put; +}; + +op_t x{0}; // OK +op_t y{.put=0}; // bogus error: 'op_t::put_t' has no non-static data member named 'put' diff --git a/gcc/testsuite/g++.dg/cpp2a/desig23.C b/gcc/testsuite/g++.dg/cpp2a/desig23.C new file mode 100644 index 00000000000..4354e644f6a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/desig23.C @@ -0,0 +1,20 @@ +// PR c++/102740 +// { dg-do compile { target c++20 } } +// { dg-additional-options -Wmissing-braces } + +typedef struct { + union { + struct { + const void* content; + } put; + }; +} op_t; + +op_t f(const char* alias) { + return op_t{ + .put = + { + .content = alias, + }, + }; // { dg-warning "missing braces" } +} diff --git a/gcc/testsuite/g++.dg/cpp2a/desig24.C b/gcc/testsuite/g++.dg/cpp2a/desig24.C new file mode 100644 index 00000000000..219cc9c3b8e --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/desig24.C @@ -0,0 +1,11 @@ +// PR c++/103299 +// { dg-do compile { target c++20 } } + +struct foo { + union { + int fp1{}; + char fp2; + }; +}; + +static_assert(foo{.fp2={}}.fp2 == 0); diff --git a/gcc/testsuite/g++.dg/cpp2a/desig25.C b/gcc/testsuite/g++.dg/cpp2a/desig25.C new file mode 100644 index 00000000000..9da958c29e9 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/desig25.C @@ -0,0 +1,13 @@ +// PR c++/102538 +// { dg-do run { target c++20 } } + +struct X { union { char r8[8]; int r32[2]; }; }; +struct Y { X v[1]; }; +Y x = { { { .r32 = { 5, 6 } } } }; + +int +main () +{ + if (x.v[0].r32[0] != 5 || x.v[0].r32[1] != 6) + __builtin_abort (); +} diff --git a/gcc/testsuite/g++.dg/ext/flexary3.C b/gcc/testsuite/g++.dg/ext/flexary3.C index 34b17254f8c..8344b42dd16 100644 --- a/gcc/testsuite/g++.dg/ext/flexary3.C +++ b/gcc/testsuite/g++.dg/ext/flexary3.C @@ -16,7 +16,7 @@ struct s { int main() { - struct s s = { .c = 0 }; // { dg-error "initializer" } + struct s s = { .c = 0 }; // { dg-error "non-static initialization of a flexible array member" "" { target *-*-* } .-1 } return 0; } diff --git a/gcc/testsuite/g++.dg/parse/pr43765.C b/gcc/testsuite/g++.dg/parse/pr43765.C index 5e602204007..aa099a4d20b 100644 --- a/gcc/testsuite/g++.dg/parse/pr43765.C +++ b/gcc/testsuite/g++.dg/parse/pr43765.C @@ -12,6 +12,6 @@ SomeType vals[] = { { 0, values : temp, }, // { dg-error "either all initializer clauses should be designated or none of them should be" "" { target c++2a } } 0 - }; // { dg-error "GNU-style designated initializer for an array|cannot convert" } -// (note the error above is on the wrong line) - // { dg-error "initialization of flexible array member in a nested context" "" { target *-*-* } .-2 } + }; +// (note the error below is on the wrong line) +// { dg-error "initialization of flexible array member in a nested context" "" { target *-*-* } .-2 }