On Wed, Apr 13, 2016 at 11:36 AM, Martin Sebor <mse...@gmail.com> wrote: > On 04/12/2016 12:17 PM, Jason Merrill wrote: >> >> On 04/10/2016 07:14 PM, Martin Sebor wrote: >>> >>> The call to build_vla_check() in check_initializer() is to check >>> only explicitly initialized VLAs. The latter function may need >>> to complete the VLA element type and reshape the initializer so >>> the call cannot be made earlier. The function returns the runtime >>> initializer code so the call cannot be made after it returns. >> >> >> But this call isn't checking the initializer; we're looking at the >> initializer in build_vec_init now. > > > Let me try to explain. Strings aside, there are three cases each > of which the patch handles in a different function: > > 1) The No Initializer case is handled by calling build_vla_check > directly from cp_finish_decl. > > 2) The Empty initializer case is handled from check_initializer > in the hunk you had pasted upthread. (It isn't handled in > build_vec_init because length_check is false for empty > initializers, and because it's too late to check VLA bounds > there anyway -- see below.) > > 3) In the Non-empty Initializer case, excess initializer elements > are checked for in build_vec_init. VLA bounds are checked from > check_initializer same as in (2). > > As I mentioned last Friday, emitting the check for the VLA bounds > in build_vec_init appears to be too late because by then the code > to allocate the stack has already been emitted. Maybe there's > a way to do it, I don't know. Controlling what piece of code is > emitted when is something I don't know much about yet. > >>> The call to build_vla_check() in cp_finish_decl() is guarded with >>> !init and made to check VLAs that are not explicitly initialized. >>> This call could perhaps be moved into check_initializer() though >>> it doesn't seem like it logically belongs there (since there is >>> no initializer). >> >> >> check_initializer handles implicit (default-) as well as explicit >> initialization. >> >>> If it were moved there, it seems to me that it >>> would require more extensive changes to the function than making >>> it in cp_finish_decl(). >> >> >> I don't see that; you ought to be able to move the check_initializer >> copy down out of the if/else structure and use that for both cases. > > > You're right, it was simpler than I thought. I was being overly > conservative in an effort to avoid changing more code than is > absolutely necessary. > > Attached is an updated patch with this simplification. It avoids > case (1) above. It also adds an argument to build_vla_check to > avoid building the size check when it's called from build_vec_init. > > I also modified the alternate patch accordingly. It's attached > for comparison. I still find it preferable to the first patch. > It's simpler because it doesn't require the special handling for > strings and avoids parameterizing build_vla_check so as not to > build the duplicate check in build_vec_init. > > I don't see the benefit in doing the checking in build_vec_init > and split_constant_init_1 when it can all be done just in > check_initializer. I'm sure you have your reasons for going that > route so I'd appreciate if you could let me know why you think > that's better.
It caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70652 -- H.J.
