On Mon, 2015-02-09 at 21:42 +0000, Jose Fonseca wrote: > On 09/02/15 21:29, Jan Vesely wrote: > > On Mon, 2015-02-09 at 19:49 +0000, Jose Fonseca wrote: > >> On 09/02/15 19:17, Ian Romanick wrote: > >>> On 02/06/2015 11:46 AM, Jose Fonseca wrote: > >>>> I haven't tried this sort of code with MSVC 2013 U4, but at least in the > >>>> past, MSVC 2013 was refusing certain kinds of variable length arrays. > >>>> > >>>> And Jan's patch is a stepping stone to -Wvla option when compiling > >>>> (option which apply to the whole tree, including parts that are not > >>>> built with MSVC), in order to preemptively spot uses of VLA in places > >>>> where MSVC would break. > >>> > >>> In Mesa, we can change compiler options in subdirectories. Is it > >>> possible to do this with cmake? Like, could we remove -Wvla in > >>> directories that are only built on Linux? It seems like that could give > >>> us the best of both worlds... > >> > >> Yes, I just confirmed that MSVC 2013 U4 still fails to compile VLA code > >> and was about to propose something similar. > >> > >> In theory this > >> > >> string (REPLACE "-Wvla" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS}) > >> > >> should undo. And it does work. But there's a minor snafu: this line > >> must be put in the tests/spec/foo/CMakeLists.txt -- if it's put in the > >> tests/spec/foo/CMakeLists.gl*.txt it has no effect -- CMAKE_C_FLAGS is > >> changed, but the source files are still build with -Wvla. > >> > >> (I'm always amazed that the CMakeLists.txt -> CMakeLists.gl*.txt system > >> works!) > > > > I think it's a good idea to keep the warnings/errors global for the sake > > of consistency. The current setup (-Wvla, -Wdeclaration-after-statement) > > only affects the parts changed in these two patches. > > There are pro/cons both ways. I'm happy to let the people that most > develop on the affected directories to decide based on their preferences. > > > Unfortunately it's not possible to tweak CMAKE_C_FLAGS on a per API > basis as I explained before. It has to be the whole directory. > > > > The errors reported below are not caught by -Wvla, since they are not > > variable length arrays. I don't think there's a gcc warning to catch > > those. > > Note the errors I pasted were with > http://cgit.freedesktop.org/piglit/commit/?id=0784e54b75918b94ac974197b736c53ccba69ff1 > > reverted. This is was newest VLA related MSVC build breakage I could find.
ah, it makes sense now, I spoke too quickly, should have verified the error codes. Anyway, all 3 patches are on the list, and given the minimal changes required I think it makes sense to go global, maybe add -Wsign-compare to the mix too. I'll wait to see what others think. thanks for your time, jan > > > Jose > > > > > jan > > > >> > >> > >>>> So let me do the following: I'll see if MSVC 2013 accepts or refuses the > >>>> VLA code that caused problems the first time, and then we'll take it > >>>> from there. > >>>> > >>>> Jose > >>> > >> > >> For the record these were MSVC errors: > >> > >> AILED: C:\PROGRA~2\MICROS~4.0\VC\bin\cl.exe /nologo /DWIN32 /D_WINDOWS > >> /W3 /D_DEBUG /MDd /Zi /Ob0 /Od /RTC1 -I..\..\src > >> -IH:\msvc32\freeglut\include -Itests\util -I..\..\tests\util > >> -IH:\noarch\glext -W3 -wd4018 -wd4244 -wd4305 -wd4800 /showIncludes > >> -DGLAPIENTRY=__stdcall -DNOMINMAX -DPIGLIT_USE_GLUT > >> -DPIGLIT_USE_GLUT_INIT_ERROR_FUNC -DPIGLIT_USE_OPENGL > >> -DWIN32_LEAN_AND_MEAN=1 -D_CRT_NONSTDC_NO_WARNINGS > >> -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GNU_SOURCE > >> -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_USE_MATH_DEFINES > >> /Fotarget_api\gl\tests\spec\gl-1.0\CMakeFiles\gl-1.0-readpixsanity.dir\readpix.c.obj > >> /Fdtarget_api\gl\tests\spec\gl-1.0\CMakeFiles\gl-1.0-readpixsanity.dir/ > >> /FS -c ..\..\tests\spec\gl-1.0\readpix.c > >> ..\..\tests\spec\gl-1.0\readpix.c(103) : error C2057: expected constant > >> expression > >> ..\..\tests\spec\gl-1.0\readpix.c(103) : error C2466: cannot allocate an > >> array of constant size 0 > >> ..\..\tests\spec\gl-1.0\readpix.c(103) : error C2087: 'buf' : missing > >> subscript > >> ..\..\tests\spec\gl-1.0\readpix.c(103) : error C2133: 'buf' : unknown size > >> ..\..\tests\spec\gl-1.0\readpix.c(200) : error C2057: expected constant > >> expression > >> ..\..\tests\spec\gl-1.0\readpix.c(200) : error C2466: cannot allocate an > >> array of constant size 0 > >> ..\..\tests\spec\gl-1.0\readpix.c(200) : error C2087: 'buf' : missing > >> subscript > >> ..\..\tests\spec\gl-1.0\readpix.c(200) : error C2133: 'buf' : unknown size > >> ..\..\tests\spec\gl-1.0\readpix.c(281) : error C2057: expected constant > >> expression > >> ..\..\tests\spec\gl-1.0\readpix.c(281) : error C2466: cannot allocate an > >> array of constant size 0 > >> ..\..\tests\spec\gl-1.0\readpix.c(281) : error C2087: 'buf' : missing > >> subscript > >> ..\..\tests\spec\gl-1.0\readpix.c(281) : error C2133: 'buf' : unknown size > >> > >> > >> So there's no doubt here: by expecting a constant expression it is clear > >> that MSVC does not support VLA at all. > >> > >> > >> Jose > > > -- Jan Vesely <jan.ves...@rutgers.edu>
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit