Checking all C warning flags at once during configuration

Checking all C warning flags at once during configuration

[Alex Gramiak]

There was recently a discussion about the long configuration step of the
build process, and I noticed that a non-negligible section involved
checking support for specific C warning flags.

Assuming that most/all of the warnings are supported by common modern
compilers, would it make sense to first check _all_ (or, at least the
commonly supported) warning flags simultaneously and then only check
them individually if there was an error?

This way the common behaviour would be sped up a bit, and the case of
unsupported flags would just be an extra AC_LINK_IFELSE slower.

Re: Checking all C warning flags at once during configuration

[Paul Eggert]

On 4/29/19 7:53 AM, Alex Gramiak wrote:
> would it make sense to first check _all_ (or, at least the
> commonly supported) warning flags simultaneously and then only check
> them individually if there was an error?

Sure. That would be a nice thing to add to Gnulib (which is the upstream
source for this).

Re: Checking all C warning flags at once during configuration

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 4/29/19 7:53 AM, Alex Gramiak wrote:
>> would it make sense to first check _all_ (or, at least the
>> commonly supported) warning flags simultaneously and then only check
>> them individually if there was an error?
>
> Sure. That would be a nice thing to add to Gnulib (which is the upstream
> source for this).

Evidently I don't know enough autoconf to be able to do this myself,
since I couldn't figure out how to implement the fallback behaviour.
I've included a diff below that implements multiple-checking (~29s to
~22s configure time for me), with a faulty loop in the else clause
commented out. Is this salvageable?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nocheck --]
[-- Type: text/x-patch, Size: 5507 bytes --]

diff --git a/configure.ac b/configure.ac
index 79fe0c98c6..0be51e0236 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1011,10 +1011,10 @@ AC_DEFUN
   AS_IF([test "$emacs_cv_clang" = yes],
    [
      # Turn off some warnings if supported.
-     gl_WARN_ADD([-Wno-switch])
-     gl_WARN_ADD([-Wno-pointer-sign])
-     gl_WARN_ADD([-Wno-string-plus-int])
-     gl_WARN_ADD([-Wno-unknown-attributes])
+     gl_WARN_ADD_NOCHECK([-Wno-switch])
+     gl_WARN_ADD_NOCHECK([-Wno-pointer-sign])
+     gl_WARN_ADD_NOCHECK([-Wno-string-plus-int])
+     gl_WARN_ADD_NOCHECK([-Wno-unknown-attributes])
    ])
  ],[
   isystem='-isystem '
@@ -1091,25 +1091,25 @@ AC_DEFUN
   gl_MANYWARN_ALL_GCC([ws])
   gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
   for w in $ws; do
-    gl_WARN_ADD([$w])
+    gl_WARN_ADD_NOCHECK([$w])
   done
-  gl_WARN_ADD([-Wredundant-decls])     # Prefer this, as we don't use Bison.
-  gl_WARN_ADD([-Wno-missing-field-initializers]) # We need this one
-  gl_WARN_ADD([-Wno-override-init])    # More trouble than it is worth
-  gl_WARN_ADD([-Wno-sign-compare])     # Too many warnings for now
-  gl_WARN_ADD([-Wno-type-limits])      # Too many warnings for now
-  gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now
-  gl_WARN_ADD([-Wno-format-nonliteral])
+  gl_WARN_ADD_NOCHECK([-Wredundant-decls])     # Prefer this, as we don't use Bison.
+  gl_WARN_ADD_NOCHECK([-Wno-missing-field-initializers]) # We need this one
+  gl_WARN_ADD_NOCHECK([-Wno-override-init])    # More trouble than it is worth
+  gl_WARN_ADD_NOCHECK([-Wno-sign-compare])     # Too many warnings for now
+  gl_WARN_ADD_NOCHECK([-Wno-type-limits])      # Too many warnings for now
+  gl_WARN_ADD_NOCHECK([-Wno-unused-parameter]) # Too many warnings for now
+  gl_WARN_ADD_NOCHECK([-Wno-format-nonliteral])
 
   # clang is unduly picky about some things.
   if test "$emacs_cv_clang" = yes; then
-    gl_WARN_ADD([-Wno-missing-braces])
-    gl_WARN_ADD([-Wno-null-pointer-arithmetic])
+    gl_WARN_ADD_NOCHECK([-Wno-missing-braces])
+    gl_WARN_ADD_NOCHECK([-Wno-null-pointer-arithmetic])
   fi
 
   # This causes too much noise in the MinGW build
   if test $opsys = mingw32; then
-    gl_WARN_ADD([-Wno-pointer-sign])
+    gl_WARN_ADD_NOCHECK([-Wno-pointer-sign])
   fi
 
   AC_DEFINE([GCC_LINT], [1], [Define to 1 if --enable-gcc-warnings.])
@@ -1127,11 +1127,13 @@ AC_DEFUN
 # clang is picky about these regardless of whether
 # --enable-gcc-warnings is specified.
 if test "$emacs_cv_clang" = yes; then
-  gl_WARN_ADD([-Wno-initializer-overrides])
-  gl_WARN_ADD([-Wno-tautological-compare])
-  gl_WARN_ADD([-Wno-tautological-constant-out-of-range-compare])
+  gl_WARN_ADD_NOCHECK([-Wno-initializer-overrides])
+  gl_WARN_ADD_NOCHECK([-Wno-tautological-compare])
+  gl_WARN_ADD_NOCHECK([-Wno-tautological-constant-out-of-range-compare])
 fi
 
+gl_COMPILER_OPTIONS_CHECK
+
 # Use a slightly smaller set of warning options for lib/.
 nw=
 nw="$nw -Wunused-macros"
diff --git a/m4/warnings.m4 b/m4/warnings.m4
index 235cac6171..4d8831cd59 100644
--- a/m4/warnings.m4
+++ b/m4/warnings.m4
@@ -49,6 +49,53 @@ AC_DEFUN
 AS_VAR_POPDEF([gl_Warn])dnl
 ])
 
+# gl_COMPILER_OPTIONS_CHECK([VARIABLE = WARN_CFLAGS/WARN_CXXFLAGS],
+#                            [PROGRAM = AC_LANG_PROGRAM()])
+# -----------------------------------------------------------------
+# Check if the compiler supports OPTIONS when compiling PROGRAM.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
+AC_DEFUN([gl_COMPILER_OPTIONS_CHECK],
+[
+dnl FIXME: gl_Warn must be used unquoted until we can assume Autoconf
+dnl 2.64 or newer.
+AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
+AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl
+AS_VAR_PUSHDEF([gl_warnings], [[WARN_]_AC_LANG_PREFIX[FLAGS_ONCE]])dnl
+gl_positives=
+for gl_warn_item in $gl_warnings; do
+  case $gl_warn_item in
+    -Wno-*) gl_positives="$gl_positives -W`expr "X$gl_warn_item" : 'X-Wno-\(.*\)'`" ;;
+    *) gl_positives="$gl_positives $gl_warn_item"
+  esac
+done
+m4_pushdef([gl_Positives], [$gl_positives])dnl
+AC_CACHE_CHECK([whether _AC_LANG compiler handles $gl_warnings], m4_defn([gl_Warn]), [
+  gl_save_compiler_FLAGS="$gl_Flags"
+  gl_AS_VAR_APPEND(m4_defn([gl_Flags]),
+    [" $gl_unknown_warnings_are_errors ]m4_defn([gl_Positives])["])
+  AC_LINK_IFELSE([m4_default([$2], [AC_LANG_PROGRAM([])])],
+                 [AS_VAR_SET(gl_Warn, [yes])],
+                 [AS_VAR_SET(gl_Warn, [no])])
+  gl_Flags="$gl_save_compiler_FLAGS"
+])
+AS_VAR_IF(gl_Warn,
+          [yes],
+          [gl_AS_VAR_APPEND(m4_if([$1], [], [[WARN_]_AC_LANG_PREFIX[FLAGS]], [[$1]]),
+            [" $gl_warnings"])],
+          dnl [for gl_warn_item in $gl_warnings; do
+          dnl    gl_WARN_ADD (gl_warn_item, [$1], [$2])
+          dnl  done
+          dnl ]
+          [echo ":("]
+          )
+m4_popdef([gl_Positives])dnl
+AS_VAR_POPDEF([gl_warnings])dnl
+AS_VAR_POPDEF([gl_Flags])dnl
+AS_VAR_POPDEF([gl_Warn])dnl
+AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])
+])
+
 # gl_UNKNOWN_WARNINGS_ARE_ERRORS
 # ------------------------------
 # Clang doesn't complain about unknown warning options unless one also
@@ -110,6 +157,11 @@ AC_DEFUN
          [AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])])dnl
 ])
 
+AC_DEFUN([gl_WARN_ADD_NOCHECK],
+[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS(]_AC_LANG[)])
+gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_]_AC_LANG_PREFIX[FLAGS_ONCE]], [[$2]]), [" $1"])
+])
+
 # Local Variables:
 # mode: autoconf
 # End:

Re: Checking all C warning flags at once during configuration

[Paul Eggert]

On 4/30/19 11:52 AM, Alex Gramiak wrote:
> Is this salvageable?

It's a start. It's probably useful info for someone needing to write a
patch. However, I expect that the patch should be against Gnulib, not
against Emacs, as all programs (not just Emacs) could use the acceleration.

Re: Checking all C warning flags at once during configuration

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 4/30/19 11:52 AM, Alex Gramiak wrote:
>> Is this salvageable?
>
> It's a start. It's probably useful info for someone needing to write a
> patch. However, I expect that the patch should be against Gnulib, not
> against Emacs, as all programs (not just Emacs) could use the acceleration.

Right, I'm just using Emacs as a testing ground first. Should I post
this to the Gnulib mailing list at this stage?

m4_errprintn tells me that for some reason $1 of gl_WARN_ADD is empty
when using it inside the loop, which doesn't make sense to me since I'm
using it the same way that it's used in configure.ac. Even trying to
pass it a constant string results in an empty first argument.

I inlined (part of) the definition of gl_WARN_ADD and somehow that makes
it work. I tested with a nonexistent flag and a combination of
gl_WARN_ADD_NOCHECK and gl_WARN_ADD; both worked as expected.

Would you happen to know why the argument is always empty in the
commented out loop?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nocheck v2 --]
[-- Type: text/x-patch, Size: 5877 bytes --]

diff --git a/configure.ac b/configure.ac
index 79fe0c98c6..cc1385710d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1011,10 +1011,10 @@ AC_DEFUN
   AS_IF([test "$emacs_cv_clang" = yes],
    [
      # Turn off some warnings if supported.
-     gl_WARN_ADD([-Wno-switch])
-     gl_WARN_ADD([-Wno-pointer-sign])
-     gl_WARN_ADD([-Wno-string-plus-int])
-     gl_WARN_ADD([-Wno-unknown-attributes])
+     gl_WARN_ADD_NOCHECK([-Wno-switch])
+     gl_WARN_ADD_NOCHECK([-Wno-pointer-sign])
+     gl_WARN_ADD_NOCHECK([-Wno-string-plus-int])
+     gl_WARN_ADD_NOCHECK([-Wno-unknown-attributes])
    ])
  ],[
   isystem='-isystem '
@@ -1091,25 +1091,25 @@ AC_DEFUN
   gl_MANYWARN_ALL_GCC([ws])
   gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
   for w in $ws; do
-    gl_WARN_ADD([$w])
+    gl_WARN_ADD_NOCHECK([$w])
   done
-  gl_WARN_ADD([-Wredundant-decls])     # Prefer this, as we don't use Bison.
-  gl_WARN_ADD([-Wno-missing-field-initializers]) # We need this one
-  gl_WARN_ADD([-Wno-override-init])    # More trouble than it is worth
-  gl_WARN_ADD([-Wno-sign-compare])     # Too many warnings for now
-  gl_WARN_ADD([-Wno-type-limits])      # Too many warnings for now
-  gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now
-  gl_WARN_ADD([-Wno-format-nonliteral])
+  gl_WARN_ADD_NOCHECK([-Wredundant-decls])     # Prefer this, as we don't use Bison.
+  gl_WARN_ADD_NOCHECK([-Wno-missing-field-initializers]) # We need this one
+  gl_WARN_ADD_NOCHECK([-Wno-override-init])    # More trouble than it is worth
+  gl_WARN_ADD_NOCHECK([-Wno-sign-compare])     # Too many warnings for now
+  gl_WARN_ADD_NOCHECK([-Wno-type-limits])      # Too many warnings for now
+  gl_WARN_ADD_NOCHECK([-Wno-unused-parameter]) # Too many warnings for now
+  gl_WARN_ADD_NOCHECK([-Wno-format-nonliteral])
 
   # clang is unduly picky about some things.
   if test "$emacs_cv_clang" = yes; then
-    gl_WARN_ADD([-Wno-missing-braces])
-    gl_WARN_ADD([-Wno-null-pointer-arithmetic])
+    gl_WARN_ADD_NOCHECK([-Wno-missing-braces])
+    gl_WARN_ADD_NOCHECK([-Wno-null-pointer-arithmetic])
   fi
 
   # This causes too much noise in the MinGW build
   if test $opsys = mingw32; then
-    gl_WARN_ADD([-Wno-pointer-sign])
+    gl_WARN_ADD_NOCHECK([-Wno-pointer-sign])
   fi
 
   AC_DEFINE([GCC_LINT], [1], [Define to 1 if --enable-gcc-warnings.])
@@ -1127,11 +1127,13 @@ AC_DEFUN
 # clang is picky about these regardless of whether
 # --enable-gcc-warnings is specified.
 if test "$emacs_cv_clang" = yes; then
-  gl_WARN_ADD([-Wno-initializer-overrides])
-  gl_WARN_ADD([-Wno-tautological-compare])
-  gl_WARN_ADD([-Wno-tautological-constant-out-of-range-compare])
+  gl_WARN_ADD_NOCHECK([-Wno-initializer-overrides])
+  gl_WARN_ADD_NOCHECK([-Wno-tautological-compare])
+  gl_WARN_ADD_NOCHECK([-Wno-tautological-constant-out-of-range-compare])
 fi
 
+gl_COMPILER_OPTIONS_CHECK_UNCHECKED
+
 # Use a slightly smaller set of warning options for lib/.
 nw=
 nw="$nw -Wunused-macros"
diff --git a/m4/warnings.m4 b/m4/warnings.m4
index 235cac6171..296bcf01c8 100644
--- a/m4/warnings.m4
+++ b/m4/warnings.m4
@@ -49,6 +49,62 @@ AC_DEFUN
 AS_VAR_POPDEF([gl_Warn])dnl
 ])
 
+# gl_COMPILER_OPTIONS_CHECK_UNCHECKED([VARIABLE = WARN_CFLAGS/WARN_CXXFLAGS],
+#                                     [PROGRAM = AC_LANG_PROGRAM()])
+# -----------------------------------------------------------------
+# Check if the compiler supports options added by gl_WARN_ADD_NOCHECK
+# when compiling PROGRAM.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
+AC_DEFUN([gl_COMPILER_OPTIONS_CHECK_UNCHECKED],
+[
+dnl FIXME: gl_Warn must be used unquoted until we can assume Autoconf
+dnl 2.64 or newer.
+AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
+AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl
+AS_VAR_PUSHDEF([gl_warnings], [[WARN_]_AC_LANG_PREFIX[FLAGS_ONCE]])dnl
+gl_positives=
+for gl_warn_item in $gl_warnings; do
+  case $gl_warn_item in
+    -Wno-*) gl_positives="$gl_positives -W`expr "X$gl_warn_item" : 'X-Wno-\(.*\)'`" ;;
+    *) gl_positives="$gl_positives $gl_warn_item"
+  esac
+done
+m4_pushdef([gl_Positives], [$gl_positives])dnl
+AC_CACHE_CHECK([whether _AC_LANG compiler handles $gl_warnings], m4_defn([gl_Warn]), [
+  gl_save_compiler_FLAGS="$gl_Flags"
+  gl_AS_VAR_APPEND(m4_defn([gl_Flags]),
+    [" $gl_unknown_warnings_are_errors ]m4_defn([gl_Positives])["])
+  AC_LINK_IFELSE([m4_default([$2], [AC_LANG_PROGRAM([])])],
+                 [AS_VAR_SET(gl_Warn, [yes])],
+                 [AS_VAR_SET(gl_Warn, [no])])
+  gl_Flags="$gl_save_compiler_FLAGS"
+])
+AS_VAR_IF(gl_Warn,
+          [yes],
+          [gl_AS_VAR_APPEND(m4_if([$1], [], [[WARN_]_AC_LANG_PREFIX[FLAGS]], [[$1]]),
+            [" $gl_warnings"])],
+          [for gl_warn_item in $gl_warnings; do
+             gl_COMPILER_OPTION_IF([$gl_warn_item],
+  [gl_AS_VAR_APPEND(m4_if([$1], [], [[WARN_]_AC_LANG_PREFIX[FLAGS]], [[$1]]), [" $gl_warn_item"])],
+  [],
+  [$2])
+           done
+          ]
+          dnl [for gl_warn_item in $gl_warnings; do
+          dnl    gl_WARN_ADD ([$gl_warn_item])
+          dnl  done
+          dnl ]
+          )
+m4_popdef([gl_Positives])dnl
+AS_VAR_POPDEF([gl_warnings])dnl
+AS_VAR_POPDEF([gl_Flags])dnl
+AS_VAR_POPDEF([gl_Warn])dnl
+m4_ifval([$1],
+         [AS_LITERAL_IF([$1], [AC_SUBST([$1])])],
+         [AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])])dnl
+])
+
 # gl_UNKNOWN_WARNINGS_ARE_ERRORS
 # ------------------------------
 # Clang doesn't complain about unknown warning options unless one also
@@ -110,6 +166,11 @@ AC_DEFUN
          [AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])])dnl
 ])
 
+AC_DEFUN([gl_WARN_ADD_NOCHECK],
+[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS(]_AC_LANG[)])
+gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_]_AC_LANG_PREFIX[FLAGS_ONCE]], [[$2]]), [" $1"])
+])
+
 # Local Variables:
 # mode: autoconf
 # End:

Re: Checking all C warning flags at once during configuration

[Paul Eggert]

On 5/1/19 11:25 AM, Alex Gramiak wrote:
> Would you happen to know why the argument is always empty in the
> commented out loop?

Sorry, not offhand by just looking at the patch.

> Should I post
> this to the Gnulib mailing list at this stage?
That's a more-appropriate list for such a patch, yes. Please try to make
your email self-contained. bug-gnulib is less populated-than emacs-devel
so I wouldn't expect an overwhelming response. What would help is a
sample use and timing statistics to motivate other readers to look into it.

Re: Checking all C warning flags at once during configuration

[Alex Gramiak]

Okay, I submitted a patch at:
https://lists.gnu.org/archive/html/bug-gnulib/2019-05/msg00000.html

Turns out that I was too used to the Emacs C style and didn't see the
space after the invocation of gl_WARN_ADD or realize that it was an
issue. Whoops.