On 8/18/2009 00:28, JonY wrote:
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.am

Updated patch attached. Suggestions welcome, especially simplifying the
AM_CFLAGS part. Testers are welcome too.

Hi,
Attached updated patch, this time using case, much cleaner than if/else tests. The "[yes], [warning_level=3]," part is probably redundant.
Index: configure.ac
===================================================================
--- configure.ac        (revision 1174)
+++ configure.ac        (working copy)
@@ -112,6 +112,42 @@
 
 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=3@:>@])],
+  [AS_CASE(["$enableval"],
+    [no],  [warning_level=0],
+    [[[0-5]]], warning_level="$enableval",
+    [warning_level=3])],
+  [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 +161,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

Reply via email to