On 8/17/2009 22:37, NightStrike wrote:
1) Put the "AC_MSG_CHECKING" at the beginning, before you start the checking.
Ok, done.
2) Features like this should be an "enable" thing, not a "with" thing. "with" things are more for external tools.
Ok, done. I was wondering about that...
3) Keep the same formatting that's elsewhere in the file, or otherwise change the whole file. For instance, look at the formatting and indentation of other HELP_STRING instances.
I hope the new indentation is better.
4) You mention CPPFLAGS in your comment, but it's not used for this kind of stuff
I was using it, but forgot to change it. Sorry, outdated comment.
5) You deal with CXXFLAGS -- Where do we use C++? We may definitely do it.. I don't remember
The variable is mainly used to allow C++ specific warning flags, not all of the warning parameters can be shared. The C++ code is in the testcase directory.
6) This line in Makefile.am is probably wrong: +am_cxxfla...@add_cxx_only_warning_flags@ @ADD_CXX_ONLY_WARNING_FLAGS@ One of those should probably be ADD_C_CXX...
Thanks for catching that.
7) This line should really be a part of the "action if given" / "action if not given" section of the option string handler, to be consistent with the rest of the file: +AS_IF([test "$warning_level" == yes], [warning_level=3], [test "$warning_level" == no], [warning_level=0]) For instance, instead of "+ [warning_level=$withval],", just operate on withval directly: [AS_IF([test "$withval" == yes], [warning_level=3],[warning_level=$withval])], Same for the action if not given, and withval = no. Also, as before, follow the formatting of the rest of the file regarding indentation.
Ok, the logic check is now included in the AC_ARG_ENABLE part.
On a more general note, I'm wondering if there isn't a better way to compute all of this in Makefile.am. Automake supports conditionals, which makes the additive nature of successive levels much more simply handled there. The idea is that you should keep configure stuff in configure, and make stuff in make.
With AM_CONDITIONALs, but the code below is the best I can come up with. if WARNLEVEL0 C_Warn0=... C_CXX_WARN0=... endif if WARNLEVEL1 C_Warn0=... C_CXX_WARN0=... endif AM_CFLAGS=$(C_Warn0) $(C_Warn1) $(C_Warn2)... If its ok, I'll use the above code in Makefile.amUpdated patch attached. Suggestions welcome, especially simplifying the AM_CFLAGS part. Testers are welcome too.
Index: configure.ac =================================================================== --- configure.ac (revision 1174) +++ configure.ac (working copy) @@ -112,6 +112,44 @@ AC_CHECK_HEADER([_mingw_mac.h], [], [AC_MSG_ERROR([Please check if the mingw-w64 header set and the build/host option are set properly.])]) +#Warnings and errors, default level is 3 +AC_MSG_CHECKING([for warning levels]) +AC_ARG_ENABLE([warnings], + [AS_HELP_STRING([[--enable-warnings[=0-5]]], + [Enable compile time warnings (default is 3)])], + [if test "$enableval" = no; then warning_level=0 + elif test "$enableval" = yes; then warning_level=3 + elif test $enableval -gt 5 || test $warning_level -lt 0; then warning_level=3 + else warning_level=$enableval + fi + ], + [warning_level=3]) +AC_MSG_RESULT([Level $warning_level]) + +#Add warning flags as appropriate. Level 4 and above are only for testing purpose. +AS_CASE([$warning_level], + [0],[ADD_C_CXX_WARNING_FLAGS="-Wall"], + [1],[ + ADD_C_CXX_WARNING_FLAGS="-Wall -Wextra" + ADD_C_ONLY_WARNING_FLAGS="-Wimplicit-function-declaration"], + [2],[ + ADD_C_CXX_WARNING_FLAGS="-Wall -Wextra -Wformat -Wstrict-aliasing -Wsystem-headers -Wshadow -Wmissing-declarations -Wpacked -Wredundant-decls -Winline" + ADD_C_ONLY_WARNING_FLAGS="-Wimplicit-function-declaration"], + [3],[ + ADD_C_CXX_WARNING_FLAGS="-Wall -Wextra -Wformat -Wstrict-aliasing -Wsystem-headers -Wshadow -Wmissing-declarations -Wpacked -Wredundant-decls -Winline -Wstrict-aliasing=3" + ADD_C_ONLY_WARNING_FLAGS="-Wimplicit-function-declaration -Wmissing-noreturn -Wmissing-prototypes"], + [4],[ + ADD_C_CXX_WARNING_FLAGS="-Wall -Wextra -Wformat -Wstrict-aliasing -Wsystem-headers -Wshadow -Wmissing-declarations -Wpacked -Wredundant-decls -Winline -Wstrict-aliasing=2 -Werror -pedantic" + ADD_C_ONLY_WARNING_FLAGS="-Wimplicit-function-declaration -Wmissing-noreturn -Wmissing-prototypes"] + [5],[ + ADD_C_CXX_WARNING_FLAGS="-Wall -Wextra -Wformat -Wstrict-aliasing -Wsystem-headers -Wshadow -Wmissing-declarations -Wpacked -Wredundant-decls -Winline -Wstrict-aliasing=2 -Werror -Wfatal-errors -pedantic -pedantic-errors" + ADD_C_ONLY_WARNING_FLAGS="-Wimplicit-function-declaration -Wmissing-noreturn -Wmissing-prototypes"] + ) + +AC_SUBST(ADD_C_ONLY_WARNING_FLAGS) +AC_SUBST(ADD_C_CXX_WARNING_FLAGS) +AC_SUBST(ADD_CXX_ONLY_WARNING_FLAGS) + AC_CONFIG_FILES([Makefile]) AC_OUTPUT @@ -125,9 +163,12 @@ AC_MSG_NOTICE([]) AC_MSG_NOTICE([Configuration Options Summary:]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ WinCE runtime.....: $enable_libce]) -AC_MSG_NOTICE([ Win32 runtime.....: $enable_lib32]) -AC_MSG_NOTICE([ Win64 runtime.....: $enable_lib64]) +AC_MSG_NOTICE([ WinCE runtime........: $enable_libce]) +AC_MSG_NOTICE([ Win32 runtime........: $enable_lib32]) +AC_MSG_NOTICE([ Win64 runtime........: $enable_lib64]) +AC_MSG_NOTICE([ C Warning Flags......: $ADD_C_ONLY_WARNING_FLAGS]) +AC_MSG_NOTICE([ C++ Warning Flags....: $ADD_CXX_ONLY_WARNING_FLAGS]) +AC_MSG_NOTICE([ Common Warning Flags.: $ADD_C_CXX_WARNING_FLAGS]) AC_MSG_NOTICE([]) Index: Makefile.am =================================================================== --- Makefile.am (revision 1174) +++ Makefile.am (working copy) @@ -20,9 +20,9 @@ withsys= endif - AM_CPPFLAGS=-D_CRTBLD $(sysincludes) -AM_CFLAGS=-pipe $(vistasupport) -Wall -Wno-strict-aliasing -std=gnu99 +AM_CFLAGS=-pipe $(vistasupport) -std=gnu99 @ADD_C_CXX_WARNING_FLAGS@ @ADD_C_ONLY_WARNING_FLAGS@ +am_cxxfla...@add_c_cxx_warning_flags@ @ADD_CXX_ONLY_WARNING_FLAGS@ AM_DLLTOOLFLAGS=-k --as=$(AS) --output-lib $@ DTDEF=$(DLLTOOL) $(AM_DLLTOOLFLAGS) --def DTLIB=$(DTDEF) $(top_srcdir)/`echo $@ | $(SED) 's|/lib|/|;s|\.a|.def|'`
------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public