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

[Jan Vesely]

On Wed, 2015-02-18 at 13:49 +0000, Jose Fonseca wrote:
> 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.

I agree, I have rebased my patches on your changes. I can push the
series as soon as there is a review, or I can repost the series to make
it clearer (not lost in a long thread).

jan

> 
> 
> Jose
> 

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150219/2834d4f1/attachment.sig>