[mapserver-dev] Mixed code and variable definition breaks MSVC builds

[Even Rouault]

Le samedi 21 juillet 2012 23:41:20, Tamas Szekeres a écrit :
> Dear Committers,
> 
> I usually find issues in the mapserver codebase due to the mixed code and
> variable definitions committed into the code according to the C++/C99 style
> coding convention. This makes the code uncompilable with MSVC which is
> still pretty much C89 with a few minor extensions, and it doesn't support
> this practice by default. We could definitely rename the corresponding c
> files to cpp or force the c++ compilation or we can use a commandine option
> to provide the same effect if we would like to follow this convention.
> 
> In the past I continouosly fixed this issue by modifying the corresponding
> code to use C89 forward definitions, but I'm not sure I'd like to take the
> responsibility do such things in a daily practice. At the moment we also
> have such issues in mapserver/master breaking the Windows builds for
> several days.
> 
> What would be the reasonable solution to prevent this issue happening so
> frequently? We should probably make a decision whether we should compile
> MapServer with MSVC for C++  or declare we might want to use forward
> definitions everywhere in the code (Probably MS RFC 7.1 should be modified
> with this).
> 
> BTW: gcc users may sometimes take a look at the daily builds at
> http://www.gisinternals.com/sdk/ to make sure thinks are working correctly
> with MSVC or not.

Based on what exists in GDAL, I suggest the following change in configure.in, 
as well as the addition of m4/ax_cflags_warn_all.m4, which will add by default 
the "-Wdeclaration-after-statement" flag in CFLAGS with GCC, which generates 
"warning: ISO C90 forbids mixed declarations and code" (what would be cool 
would be to error out, only on that error, instead of just warning)

maputil.c: In function 'msOffsetPolyline':
maputil.c:1748: warning: ISO C90 forbids mixed declarations and code
maplabel.c: In function 'msAddLabelGroup':
maplabel.c:361: warning: ISO C90 forbids mixed declarations and code
maplabel.c: In function 'msAddLabel':
maplabel.c:552: warning: ISO C90 forbids mixed declarations and code
maplabel.c:576: warning: ISO C90 forbids mixed declarations and code
mapresample.c: In function 'msNearestRasterResampler':
mapresample.c:119: warning: unused variable 'nValue'
mapresample.c: In function 'msSourceSample':
mapresample.c:272: warning: ISO C90 forbids mixed declarations and code
mapresample.c: In function 'msBilinearRasterResampler':
mapresample.c:445: warning: ISO C90 forbids mixed declarations and code
mapdraw.c: In function 'msDrawVectorLayer':
mapdraw.c:1043: warning: ISO C90 forbids mixed declarations and code


diff --git a/configure.in b/configure.in
index 92c94b5..304b7b7 100755
--- a/configure.in
+++ b/configure.in
@@ -36,6 +36,7 @@ m4_include([m4/mapserver.m4])
 m4_include([m4/apache.m4])
 m4_include([m4/ax_jni_include_dir.m4])
 m4_include([m4/ax_pkg_swig.m4])
+m4_include([m4/ax_cflags_warn_all.m4])
 
 AC_CONFIG_SRCDIR([mapserv.c])
 dnl Checks for programs.
@@ -2162,42 +2163,33 @@ AC_MSG_CHECKING(compiler warnings)
 AC_ARG_WITH(warnings,
 [  --with-warnings[[=flags]] Enable strict warnings (or user defined 
warnings)],,)
 
+AX_CFLAGS_WARN_ALL(C_WFLAGS)
+AX_CXXFLAGS_WARN_ALL(CXX_WFLAGS)
+
 if test "$with_warnings" = "yes" ; then 
   if test "$GCC" = "yes" ; then
-    WFLAGS="-W -Wall -Wcast-align -Wmissing-prototypes -Wstrict-prototypes -
Wpointer-arith -Wreturn-type"
-    C_EXTRA_WFLAGS="-Wmissing-declarations"
+    WFLAGS="-W -Wcast-align -Wmissing-prototypes -Wstrict-prototypes -
Wpointer-arith -Wreturn-type"
+    C_WFLAGS="$C_WFLAGS -Wmissing-declarations"
   
     AC_MSG_RESULT([strict])
   else
     WFLAGS=""
-    C_EXTRA_WFLAGS=""
-    AC_MSG_RESULT([disabled])
-  fi
-
-elif test "$with_warnings" = "" ; then 
-  if test "$GCC" = "yes" ; then
-    WFLAGS="-Wall"
-    C_EXTRA_WFLAGS=""
-    AC_MSG_RESULT([basic])
-  else
-    WFLAGS=""
-    C_EXTRA_WFLAGS=""
     AC_MSG_RESULT([disabled])
   fi
 
 elif test "$with_warnings" = "no" ; then
   WFLAGS=""
-  C_EXTRA_WFLAGS=""
+  C_WFLAGS=""
+  CXX_WFLAGS=""
   AC_MSG_RESULT([disabled])
 
-else
+elif test "$with_warnings" != "" ; then 
   WFLAGS="$with_warnings"
-  C_EXTRA_WFLAGS=""
   AC_MSG_RESULT([user defined])
 fi
 
-CFLAGS="$CFLAGS $WFLAGS $C_EXTRA_WFLAGS"
-CXXFLAGS="$CXXFLAGS $WFLAGS "
+CFLAGS="$CFLAGS $WFLAGS $C_WFLAGS"
+CXXFLAGS="$CXXFLAGS $WFLAGS $CXX_WFLAGS"
 
 dnl ---------------------------------------------------------------------
 dnl Check --enable-debug option for "-g" compile flag. (OFF by default)

> 
> Best regards,
> 
> Tamas