[Piglit] [PATCH 2/3] Use alloca instead of variable length arrays

[Jose Fonseca]

On 12/02/15 18:51, Jan Vesely wrote:
> On Thu, 2015-02-12 at 11:17 +0000, Emil Velikov wrote:
>> On 11 February 2015 at 16:12, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>>> On Tue, 2015-02-10 at 10:28 -0800, Dylan Baker wrote:
>>>> I just want to be clear I was asking a question, I don't really care one
>>>> way or another, I would just rather not see code churn if it doesn't
>>>> actually buy us anything.
>>>
>>> Not sure what the question is here. The idea is to force msvc like
>>> limitations on other compilers by using -Werror= for unsupported
>>> features. It should result in fewer commits like [0].
>>> We can do it
>>> a) globally even for stuff that is never built using msvc
>>> b) per directory
>>>
>>> I think a) is better given that the required changes are minimal (only
>>> the two posted patches), and using alloca instead gives identical
>>> behavior. it makes codestyle consistent across files, and Jose's way of
>>> removing the flags using string function seems a bit hacky to me
>>>
>> Do you have a rough number how many tests warn about VLA currently ?
>> Must admit that I've very rarely look at the compilation output of
>> piglit. That combined the fact that new piglits get added
>> incrementally is a nice indication, imho, about one should handle
>> this.
>>
>> Or in other words - it there are only a few of tests that need fixing,
>> there should be no problem with adding this. Otherwise it's a
>> different story.
>
> When configured with all PIGLIT_BUILD_*_TESTS set to ON only the two
> patches in this series (1/3, 2/3) are needed to make the entire tree
> (0060f5998a) build with -Werror=declaration-after-statement -Werror=vla
> (gcc 4.9.2, although I don't think these are version dependent)

I don't think we need the declaration-after-statement anymore.  There 
were some bugs in MSVC 2013 prior to Update 4 that gave the impression 
it wasn't supported, but it seems to work reasonably with Update 4. 
I've seen Waffle using it.


Concerning, the VLA, it doesn't look like anybody feels particularly 
strong either way.

Given it's just a handful of cases, and you already made things working, 
while enabling/disabling per-directory is more work, I say let's just go 
with your patches and move on.


Jose