Message ID | 20161208202832.GK2337@redhat.com |
---|---|
State | New |
Headers | show |
On 12/08/2016 01:28 PM, Marek Polacek wrote: > On Thu, Dec 08, 2016 at 02:56:56PM -0500, Nathan Sidwell wrote: >> On 12/08/2016 01:05 PM, Jason Merrill wrote: >>> If the problem is the member initializer, we can diagnose that >>> directly rather than wait until we're in a constructor. >> >> What about: >> struct Foo { >> int a; >> char ary[]; >> Foo () : ary ("bob"){} >> }; >> >> ? That ICEs in the same way. > > My patch would make GCC to reject this too, but the new 'else' needs to > be removed from the patch, i.e. Clang accepts this test case although it does reject the originally submitted test case with the messages below. I think GCC should accept the latter case for compatibility. If it is accepted then rejecting the original test case will make the array initialization irregular. I think it would be nice if they both could accepted but I don't know how much work it would be to make it work. Martin t.C:4:12: warning: in-class initialization of non-static data member is a C++11 extension [-Wc++11-extensions] char a[] = "foo"; ^ t.C:4:12: error: array bound cannot be deduced from an in-class initializer t.C:3:8: warning: private field 'b' is not used [-Wunused-private-field] bool b; ^ t.C:4:8: warning: private field 'a' is not used [-Wunused-private-field] char a[] = "foo"; ^ 3 warnings and 1 error generated.
On Thu, Dec 08, 2016 at 01:42:44PM -0700, Martin Sebor wrote: > On 12/08/2016 01:28 PM, Marek Polacek wrote: > > On Thu, Dec 08, 2016 at 02:56:56PM -0500, Nathan Sidwell wrote: > > > On 12/08/2016 01:05 PM, Jason Merrill wrote: > > > > If the problem is the member initializer, we can diagnose that > > > > directly rather than wait until we're in a constructor. > > > > > > What about: > > > struct Foo { > > > int a; > > > char ary[]; > > > Foo () : ary ("bob"){} > > > }; > > > > > > ? That ICEs in the same way. > > > > My patch would make GCC to reject this too, but the new 'else' needs to > > be removed from the patch, i.e. > > Clang accepts this test case although it does reject the originally > submitted test case with the messages below. I think GCC should > accept the latter case for compatibility. If it is accepted then > rejecting the original test case will make the array initialization > irregular. I think it would be nice if they both could accepted > but I don't know how much work it would be to make it work. It's likely late for that in any case. I'd think we should fix the ICEs, reject both, and maybe revisit the second case for GCC 8. Marek
On 12/08/2016 03:42 PM, Martin Sebor wrote: > On 12/08/2016 01:28 PM, Marek Polacek wrote: >> On Thu, Dec 08, 2016 at 02:56:56PM -0500, Nathan Sidwell wrote: >>> struct Foo { >>> int a; >>> char ary[]; >>> Foo () : ary ("bob"){} > Clang accepts this test case although it does reject the originally > submitted test case with the messages below. I think GCC should > accept the latter case for compatibility. If it is accepted then > rejecting the original test case will make the array initialization > irregular. I think it would be nice if they both could accepted > but I don't know how much work it would be to make it work. I think all these should be rejected for the same reason -- they're equally unmeaningfull. The string being used must be a string literal -- not a pointer to a caller-provided string, or template argument (which I could see as plausible use cases). So the problem is being baked into the class or ctor definition. nathan
On 12/09/2016 05:34 AM, Nathan Sidwell wrote: > On 12/08/2016 03:42 PM, Martin Sebor wrote: >> On 12/08/2016 01:28 PM, Marek Polacek wrote: >>> On Thu, Dec 08, 2016 at 02:56:56PM -0500, Nathan Sidwell wrote: > >>>> struct Foo { >>>> int a; >>>> char ary[]; >>>> Foo () : ary ("bob"){} > >> Clang accepts this test case although it does reject the originally >> submitted test case with the messages below. I think GCC should >> accept the latter case for compatibility. If it is accepted then >> rejecting the original test case will make the array initialization >> irregular. I think it would be nice if they both could accepted >> but I don't know how much work it would be to make it work. > > I think all these should be rejected for the same reason -- they're > equally unmeaningfull. The string being used must be a string literal > -- not a pointer to a caller-provided string, or template argument > (which I could see as plausible use cases). So the problem is being > baked into the class or ctor definition. I'm not sure I understand what you mean. Since the following is valid and meaningful (i.e., initializes the array with the elements of the string): struct Foo { int a; char ary[4]; Foo () : ary ("bob") {} }; then (IMO) so is this: struct Foo { int a; char ary[]; Foo () : ary ("bob") {} }; Martin PS I do agree that it's probably too late to make this work for GCC 7 and that rejecting it is better than an ICE.
On Fri, Dec 09, 2016 at 08:04:11AM -0700, Martin Sebor wrote: > I'm not sure I understand what you mean. Since the following is > valid and meaningful (i.e., initializes the array with the elements > of the string): > > struct Foo { > int a; > char ary[4]; > Foo () : ary ("bob") {} > }; Sure, that has a clear meaning. > then (IMO) so is this: > > struct Foo { > int a; > char ary[]; > Foo () : ary ("bob") {} > }; So what does this mean? Assume that the Foo object lives on the heap and has been allocated with extra space after it? Or shall global/automatic objects of type Foo be allocated with the extra space (like struct Foo x = { 5, "bob" }; in C would do if the Foo () ... line is removed? Something different? Jakub
On 12/09/2016 08:09 AM, Jakub Jelinek wrote: > On Fri, Dec 09, 2016 at 08:04:11AM -0700, Martin Sebor wrote: >> I'm not sure I understand what you mean. Since the following is >> valid and meaningful (i.e., initializes the array with the elements >> of the string): >> >> struct Foo { >> int a; >> char ary[4]; >> Foo () : ary ("bob") {} >> }; > > Sure, that has a clear meaning. > >> then (IMO) so is this: >> >> struct Foo { >> int a; >> char ary[]; >> Foo () : ary ("bob") {} >> }; > > So what does this mean? Assume that the Foo object lives on the heap > and has been allocated with extra space after it? Or shall global/automatic > objects of type Foo be allocated with the extra space (like > struct Foo x = { 5, "bob" }; in C would do if the Foo () ... line is > removed? > Something different? It means the same thing as the first case, down to sizeof (Foo). All of Foo's ctors would have to agree on the size of the initializer (with an error if they didn't and could be seen in the same translation unit). In most cases the notation from the bug report would then be the preferred one to use. struct Foo { int a; char ary[] = "bob"; Foo () { } }; Martin
On 12/09/2016 10:18 AM, Martin Sebor wrote: > On 12/09/2016 08:09 AM, Jakub Jelinek wrote: >> On Fri, Dec 09, 2016 at 08:04:11AM -0700, Martin Sebor wrote: >>> then (IMO) so is this: >>> >>> struct Foo { >>> int a; >>> char ary[]; >>> Foo () : ary ("bob") {} >>> }; >> >> So what does this mean? Assume that the Foo object lives on the heap >> and has been allocated with extra space after it? Or shall >> global/automatic >> objects of type Foo be allocated with the extra space (like >> struct Foo x = { 5, "bob" }; in C would do if the Foo () ... line is >> removed? >> Something different? > > It means the same thing as the first case, down to sizeof (Foo). > All of Foo's ctors would have to agree on the size of the initializer 1) if the ctors are not defined in the structure definition, how will this interact with separate compilation? (all the ctor definitions might be in TUs not visible from the TU calling them) 2) why distinguish this case from the NSDMI case? NSDMI is essentially syntactic sugar to avoid repeating the ary member init in each ctor. 3) where in C++ is the layout of an object determined by anything other than its data member definitions? nathan
On 12/09/2016 08:34 AM, Nathan Sidwell wrote: > On 12/09/2016 10:18 AM, Martin Sebor wrote: >> On 12/09/2016 08:09 AM, Jakub Jelinek wrote: >>> On Fri, Dec 09, 2016 at 08:04:11AM -0700, Martin Sebor wrote: > >>>> then (IMO) so is this: >>>> >>>> struct Foo { >>>> int a; >>>> char ary[]; >>>> Foo () : ary ("bob") {} >>>> }; >>> >>> So what does this mean? Assume that the Foo object lives on the heap >>> and has been allocated with extra space after it? Or shall >>> global/automatic >>> objects of type Foo be allocated with the extra space (like >>> struct Foo x = { 5, "bob" }; in C would do if the Foo () ... line is >>> removed? >>> Something different? >> >> It means the same thing as the first case, down to sizeof (Foo). >> All of Foo's ctors would have to agree on the size of the initializer > > 1) if the ctors are not defined in the structure definition, how will > this interact with separate compilation? (all the ctor definitions might > be in TUs not visible from the TU calling them) Right. We would have to decide how to deal with it. One way might be to error out/warn if not all the ctors were visible and the array were not (also) initialized in the class, to avoid undefined behavior. > 2) why distinguish this case from the NSDMI case? NSDMI is essentially > syntactic sugar to avoid repeating the ary member init in each ctor. Agreed. I'm suggesting the opposite: i.e., to treat these cases the same, with the NSDMI syntax being preferred and the ctor member initializer syntax being subject to some reasonable limitations to avoid undefined behavior. > 3) where in C++ is the layout of an object determined by anything other > than its data member definitions? Off the top of my head I can't think of any existing cases but one was proposed(*) that allowed struct Foo { auto i = 0; auto a = "abc"; }; For flexible array members, because they're not in C++, we get to make up the rules that make the most sense to us. IMO, they should fit in well with the rest of the language. Because the bound of a non-member array can be omitted and its size determined from its initializer, extending the same rule to [flexible] array members feels natural to me (but YMMV). Martin [*] The feature ended up getting removed from the NSDMI proposal, IIRC, out of concern about making it all too easy to change the layout of a class and violating the ODR. (The same ODR concern could be raised with non-member uses of auto, or with any number of other C++ features.)
On Fri, Dec 9, 2016 at 11:33 AM, Martin Sebor <msebor@gmail.com> wrote: > For flexible array members, because they're not in C++, we get to > make up the rules that make the most sense to us. IMO, they should > fit in well with the rest of the language. I disagree; we should support C code, but flexible arrays don't really fit with the C++ object model, so I don't think trying to do anything clever with them in constructors is worthwhile. Jason
On 12/09/2016 02:49 PM, Jason Merrill wrote: > On Fri, Dec 9, 2016 at 11:33 AM, Martin Sebor <msebor@gmail.com> wrote: >> For flexible array members, because they're not in C++, we get to >> make up the rules that make the most sense to us. IMO, they should >> fit in well with the rest of the language. > > I disagree; we should support C code, but flexible arrays don't really > fit with the C++ object model, so I don't think trying to do anything > clever with them in constructors is worthwhile. With the suggested approach the array becomes just an ordinary member. It's not a flexible array member anymore because its bound is deduced from the initializer (it just looks like one). The NSDMI char a[] = "foo" syntax is just a shorthand for char a[4] = "foo". The only open question I can think of is what to do about the (IMO) unlikely corner case where the array isn't initialized in the class but rather in two or more ctors that are defined in their own translation units: struct S { char a[]; S (int); S (int, int); }; // in foo.C S::S (int): a ("1") { } // in bar.C S::S (int, int): a ("12") { } I think warning on it or even rejecting it would be fine. This approach would make "flexible array members" (or their C++ lookalikes as per the above) safer to use in classes with ctors than the currently accepted alternative because it would let GCC detect and diagnose overflows that can't be detected without knowing the array size: S::S (int) { strcpy (a, "1234"); } Martin
On 12/09/2016 07:46 PM, Martin Sebor wrote: > On 12/09/2016 02:49 PM, Jason Merrill wrote: >> On Fri, Dec 9, 2016 at 11:33 AM, Martin Sebor <msebor@gmail.com> wrote: >>> For flexible array members, because they're not in C++, we get to >>> make up the rules that make the most sense to us. IMO, they should >>> fit in well with the rest of the language. >> >> I disagree; we should support C code, but flexible arrays don't really >> fit with the C++ object model, so I don't think trying to do anything >> clever with them in constructors is worthwhile. > > With the suggested approach the array becomes just an ordinary member. > It's not a flexible array member anymore because its bound is deduced > from the initializer (it just looks like one). The NSDMI char a[] = > "foo" syntax is just a shorthand for char a[4] = "foo". That would mean that the size of the class varies with its initializer, which again gets into not fitting with the C++ object model. Jason
On 12/13/2016 12:06 PM, Jason Merrill wrote: > On 12/09/2016 07:46 PM, Martin Sebor wrote: >> On 12/09/2016 02:49 PM, Jason Merrill wrote: >>> On Fri, Dec 9, 2016 at 11:33 AM, Martin Sebor <msebor@gmail.com> wrote: >>>> For flexible array members, because they're not in C++, we get to >>>> make up the rules that make the most sense to us. IMO, they should >>>> fit in well with the rest of the language. >>> >>> I disagree; we should support C code, but flexible arrays don't really >>> fit with the C++ object model, so I don't think trying to do anything >>> clever with them in constructors is worthwhile. >> >> With the suggested approach the array becomes just an ordinary member. >> It's not a flexible array member anymore because its bound is deduced >> from the initializer (it just looks like one). The NSDMI char a[] = >> "foo" syntax is just a shorthand for char a[4] = "foo". > > That would mean that the size of the class varies with its initializer, > which again gets into not fitting with the C++ object model. Okay, I accept the challenge of finding a case where the size of a class depends on an initializer expression :) Here's one: constexpr long f (int) { return 1; } constexpr long f (long) { return LONG_MAX; }; struct A { // sizeof (A) == 4 enum E { i = f (0) } e; }; struct B { // sizeof (B) == 8 enum E { i = f (0L) } e; }; It's not exactly the same but I think it's close enough to argue that deducing the array size from its initializer wouldn't go against the established model. FWIW, I have a (vague) recollection of an incident where the enum size problem caused ABI breakage in a product (it was in C and involved simple macros). Because an enum is normally expected to be the same size as an int, having its size change based on the (hidden behind a macro) value of one of its enumerators is likely far more unexpected than having an array size depend on its its initializer. Martin
On 12/13/2016 03:25 PM, Martin Sebor wrote: > On 12/13/2016 12:06 PM, Jason Merrill wrote: >> On 12/09/2016 07:46 PM, Martin Sebor wrote: >>> On 12/09/2016 02:49 PM, Jason Merrill wrote: >>>> On Fri, Dec 9, 2016 at 11:33 AM, Martin Sebor <msebor@gmail.com> wrote: >>>>> For flexible array members, because they're not in C++, we get to >>>>> make up the rules that make the most sense to us. IMO, they should >>>>> fit in well with the rest of the language. >>>> >>>> I disagree; we should support C code, but flexible arrays don't really >>>> fit with the C++ object model, so I don't think trying to do anything >>>> clever with them in constructors is worthwhile. >>> >>> With the suggested approach the array becomes just an ordinary member. >>> It's not a flexible array member anymore because its bound is deduced >>> from the initializer (it just looks like one). The NSDMI char a[] = >>> "foo" syntax is just a shorthand for char a[4] = "foo". >> >> That would mean that the size of the class varies with its initializer, >> which again gets into not fitting with the C++ object model. > > Okay, I accept the challenge of finding a case where the size > of a class depends on an initializer expression :) > > Here's one: > > constexpr long f (int) { return 1; } > constexpr long f (long) { return LONG_MAX; }; > > struct A { // sizeof (A) == 4 > enum E { i = f (0) } e; > }; > > struct B { // sizeof (B) == 8 > enum E { i = f (0L) } e; > }; > > It's not exactly the same but I think it's close enough to argue > that deducing the array size from its initializer wouldn't go > against the established model. I don't see the similarity; A and B both have constant size. Jason
On 12/13/2016 02:32 PM, Jason Merrill wrote: > On 12/13/2016 03:25 PM, Martin Sebor wrote: >> On 12/13/2016 12:06 PM, Jason Merrill wrote: >>> On 12/09/2016 07:46 PM, Martin Sebor wrote: >>>> On 12/09/2016 02:49 PM, Jason Merrill wrote: >>>>> On Fri, Dec 9, 2016 at 11:33 AM, Martin Sebor <msebor@gmail.com> >>>>> wrote: >>>>>> For flexible array members, because they're not in C++, we get to >>>>>> make up the rules that make the most sense to us. IMO, they should >>>>>> fit in well with the rest of the language. >>>>> >>>>> I disagree; we should support C code, but flexible arrays don't really >>>>> fit with the C++ object model, so I don't think trying to do anything >>>>> clever with them in constructors is worthwhile. >>>> >>>> With the suggested approach the array becomes just an ordinary member. >>>> It's not a flexible array member anymore because its bound is deduced >>>> from the initializer (it just looks like one). The NSDMI char a[] = >>>> "foo" syntax is just a shorthand for char a[4] = "foo". >>> >>> That would mean that the size of the class varies with its initializer, >>> which again gets into not fitting with the C++ object model. >> >> Okay, I accept the challenge of finding a case where the size >> of a class depends on an initializer expression :) >> >> Here's one: >> >> constexpr long f (int) { return 1; } >> constexpr long f (long) { return LONG_MAX; }; >> >> struct A { // sizeof (A) == 4 >> enum E { i = f (0) } e; >> }; >> >> struct B { // sizeof (B) == 8 >> enum E { i = f (0L) } e; >> }; >> >> It's not exactly the same but I think it's close enough to argue >> that deducing the array size from its initializer wouldn't go >> against the established model. > > I don't see the similarity; A and B both have constant size. Sure, just as C and D below do today struct C { // sizeof (C) == 4 char a[4] = "abc"; }; struct D { // sizeof (D) == 8 char a[8] = "abcdefg"; }; and as would E and F below with what I suggest: struct E { // sizeof (E) == 4 char a[] = "abc"; }; struct F { // sizeof (F) == 8 char a[] = "abcdefg"; }; Where is the disconnect? I'm not suggesting that the size of an object of a class change depending on how the member array is initialized in each ctor initializer list (if that's how it came across). Martin
diff --git gcc/cp/init.c gcc/cp/init.c index b4b6cdb..01009c9 100644 --- gcc/cp/init.c +++ gcc/cp/init.c @@ -800,6 +800,10 @@ perform_member_init (tree member, tree init) in that case. */ init = build_x_compound_expr_from_list (init, ELK_MEM_INIT, tf_warning_or_error); + if (TREE_CODE (type) == ARRAY_TYPE + && TYPE_DOMAIN (type) == NULL_TREE) + error_at (DECL_SOURCE_LOCATION (member), + "initialization of a flexible array member in a constructor"); if (init) finish_expr_stmt (cp_build_modify_expr (input_location, decl, diff --git gcc/testsuite/g++.dg/ext/flexary20.C gcc/testsuite/g++.dg/ext/flexary20.C index e69de29..ff97b06 100644 --- gcc/testsuite/g++.dg/ext/flexary20.C +++ gcc/testsuite/g++.dg/ext/flexary20.C @@ -0,0 +1,49 @@ +// PR c++/72775 +// { dg-do compile { target c++11 } } +// { dg-options -Wno-pedantic } + +struct S { + int i; + char a[] = "foo"; // { dg-error "initialization of a flexible array member in a constructor" } + S () {} +}; + +struct T { + int i; + char a[] = "foo"; // { dg-error "initialization of a flexible array member in a constructor" } +}; + +struct U { + int i; + char a[] = "foo"; // { dg-error "initialization of a flexible array member in a constructor" } + U (); +}; + +U::U() {} + +int +main () +{ + struct T t; +} + +struct V { + int i; + struct W { + int j; + char a[] = "foo"; // { dg-error "initialization of a flexible array member in a constructor" } + } w; + V () {} +}; + +template <class T> +struct X { + int i; + T a[] = "foo"; // { dg-error "initialization of a flexible array member in a constructor" } +}; + +void +fn () +{ + struct X<char> x; +}