On 27/02/15 15:59, Ian Romanick wrote: > I like the idea as it should prevent future thrash. There are a couple > comments below. > True. Not to mention that having -Werror=pointer-arith and -Werror=declaration-after-statement does not sounds too bad either.
Although I would suspect that either one might cause a bit of chaos atm - i.e. we do (ab)use them rather often. > On 02/26/2015 08:51 AM, Jose Fonseca wrote: >> The main objective of this change is to enable Linux developers to use >> more of C99 throughout Mesa, with confidence that the portions that need >> to be built with MSVC -- and only those portions --, stay portable. >> >> This is achieved by using the appropriate -Werror= options only on the >> places they need to be used. >> >> Unfortunately we still need MSVC 2008 on a few portions of the code >> (namely llvmpipe and its dependencies). I hope to eventually eliminate >> this so that we can use C99 everywhere, but there are technical/logistic >> challenges (specifically, newer Windows SDKs no longer bundle MSVC, >> instead require a full installation of Visual Studio, and that has >> hindered adoption of newer MSVC versions on our build processes.) >> Thankfully we have more directy control over our OpenGL driver, which is >> why we're now able to migrate to MSVC 2013 for most of the tree. >> --- >> configure.ac | 17 +++++++++++++++++ >> src/egl/main/Makefile.am | 1 + >> src/gallium/auxiliary/Makefile.am | 7 +++++-- >> src/gallium/drivers/llvmpipe/Makefile.am | 6 ++++-- >> src/gallium/state_trackers/egl/Makefile.am | 3 ++- >> src/gallium/targets/egl-static/Makefile.am | 3 ++- >> src/glsl/Makefile.am | 8 ++++++-- >> src/loader/Makefile.am | 1 + >> src/mapi/Makefile.am | 4 +++- >> src/mesa/Makefile.am | 10 ++++++++-- >> src/util/Makefile.am | 3 ++- >> 11 files changed, 51 insertions(+), 12 deletions(-) >> >> diff --git a/configure.ac b/configure.ac >> index 5fbb7bc..22dc023 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -263,6 +263,18 @@ if test "x$GCC" = xyes; then >> # gcc's builtin memcmp is slower than glibc's >> # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43052 >> CFLAGS="$CFLAGS -fno-builtin-memcmp" >> + >> + # Flags to help ensure that certain portions of the code -- and only >> those >> + # portions -- can be built with MSVC: >> + # - src/util, src/gallium/auxiliary, and src/gallium/drivers/llvmpipe >> needs >> + # to build with Windows SDK 7.0.7600, which bundles MSVC 2008 >> + # - non-Linux/Posix OpenGL portions needs to build on MSVC 2013 (which >> + # supports most of C99) >> + # - the rest has no compiler compiler restrictions >> + MSVC2013_COMPAT_CFLAGS="-Werror=vla -Werror=pointer-arith" >> + MSVC2013_COMPAT_CXXFLAGS="-Werror=vla -Werror=pointer-arith" >> + MSVC2008_COMPAT_CFLAGS="$MSVC2013_COMPAT_CFLAGS >> -Werror=declaration-after-statement" >> + MSVC2008_COMPAT_CXXFLAGS="$MSVC2013_COMPAT_CXXFLAGS" Jose, I'm guessing that we might want to move the CXXFLAGS within the GXX check below. Or maybe compact the two conditionals, as they are almost identical - if you're feeling bored, that is :) ... >> diff --git a/src/loader/Makefile.am b/src/loader/Makefile.am >> index 36ddba8..3d32279 100644 >> --- a/src/loader/Makefile.am >> +++ b/src/loader/Makefile.am >> @@ -30,6 +30,7 @@ libloader_la_CPPFLAGS = \ >> -I$(top_srcdir)/include \ >> -I$(top_srcdir)/src \ >> $(VISIBILITY_CFLAGS) \ >> + $(MSVC2013_COMPAT_CFLAGS) \ >> $(LIBUDEV_CFLAGS) > > I don't think this is necessary for src/loader. Isn't that only built > for DRI? > There are quite a few users actually src/egl/drivers/dri2/Makefile.am src/gallium/auxiliary/pipe-loader/Makefile.am src/gallium/targets/*/Makefile.am src/gbm/Makefile.am src/glx/Makefile.am ... >> diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am >> index b6cb8f1..5e9a820 100644 >> --- a/src/mesa/Makefile.am >> +++ b/src/mesa/Makefile.am >> @@ -136,8 +136,14 @@ noinst_LTLIBRARIES += libmesagallium.la >> endif >> >> AM_CPPFLAGS = $(DEFINES) $(INCLUDE_DIRS) >> -AM_CFLAGS = $(LLVM_CFLAGS) $(VISIBILITY_CFLAGS) >> -AM_CXXFLAGS = $(LLVM_CFLAGS) $(VISIBILITY_CXXFLAGS) >> +AM_CFLAGS = \ >> + $(LLVM_CFLAGS) \ >> + $(VISIBILITY_CFLAGS) \ >> + $(MSVC2013_COMPAT_CFLAGS) >> +AM_CXXFLAGS = \ >> + $(LLVM_CFLAGS) \ >> + $(VISIBILITY_CXXFLAGS) \ >> + $(MSVC2013_COMPAT_CXXFLAGS) > > Forgive my ignorance about the build system... does applying this here > affect building files down in src/mesa/drivers/dri? > It affects only the objects (_PROGRAMS, _LTLIBRARIES) built within this Makefile. Namely: libmesa_sse41.la libmesa.la libmesagallium.la gen_matypes Those are used in many places, but at that stage the above CFLAGS are irrelevant. -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
