On Fri, 3 Feb 2023, Qing Zhao wrote: > > > > On Feb 3, 2023, at 2:49 AM, Richard Biener <rguent...@suse.de> wrote: > > > > On Thu, 2 Feb 2023, Qing Zhao wrote: > > > >> > >> > >>> On Feb 2, 2023, at 8:54 AM, Richard Biener <rguent...@suse.de> wrote: > >>> > >>> On Thu, 2 Feb 2023, Qing Zhao wrote: > >>> > >>>> > >>>> > > > > [...] > > > >>>>>>>> + return flexible_size_type_p (TREE_TYPE (last)); > >>>>>>> > >>>>>>> For types with many members this can become quite slow (IIRC we had > >>>>>>> bugs about similar walks of all fields in types), and this function > >>>>>>> looks like it's invoked multiple times on the same type per TU. > >>>>>>> > >>>>>>> In principle the property is fixed at the time we lay out a record > >>>>>>> type, so we might want to compute it at that time and record the > >>>>>>> result. > >>>>>> > >>>>>> You mean in FE? > >>>>> > >>>>> Yes, either in the frontend or in the middle-ends layout_type. > >>>>> > >>>>>> Yes, that?s better and cleaner. > >>>>>> > >>>>>> I will add one more field in the TYPE structure to record this > >>>>>> information and check this field during middle end. > >>>>>> > >>>>>> I had the same thought in the beginning, but not sure whether adding a > >>>>>> new field in IR is necessary or not, other places in middle end might > >>>>>> not use this new field. > >>>>> > >>>>> It might be interesting to search for other code walking all fields of > >>>>> a type to determine this or similar info. > >>>> > >>>> There is one which is defined in tree.cc but only is referenced in > >>>> c/c-decl.cc: > >>>> > >>>> /* Determine whether TYPE is a structure with a flexible array member, > >>>> or a union containing such a structure (possibly recursively). */ > >>>> flexible_array_type_p > >>>> > >>>> However, this routine is a little different than the one I tried to add: > >>>> > >>>> In the current routine ?flexible_array_type_p?, only one level nesting > >>>> in the structure is accepted, multiple nesting in structure is not > >>>> permitted. > >>>> > >>>> So, my question is: shall we accept multiple nesting in structure? i.e. > >>> > >>> If we don't reject the testcase with an error, then yes. > >> > >> Gcc currently accepts the multiple nesting in structure without error. > >> So, we will continue to accept such extension as long as the flex array > >> is at the end of the structure. At the same time, for the case the flex > >> array is in the middle of the structure, issue additional warnings now > >> to discourage such usage, and deprecate this case in a future release. > >> > >> Does this sound reasonable? > > > > Please don't mix several issues - I think the flex array in the > > middle of a structure is separate and we shouldn't report that > > as flexible_array_type_p or flexible_size_type_p since the size > > of the containing structure is not variable. > Agreed on this. > > My major question here is (for documentation change, sorry for mixing > this thread with the documentation change): do we need to document this > case together with the case in which struct with flex array is embedded > into another structure? (As a GCC extension?)
I think this should be Josephs call - documenting this might encourage people to use such an extension, even if it's a bad one we want to get rid of. Maybe the easiest thing is to come up with a patch documenting it which we can then turn into a deprecation note depending on this outcome. Richard.
