diff mbox series

[4/4] Turn on -Wimplicit-fallthrough by default if available

Message ID 2a5b17ba992abd2311da130c18a531f89c21ee79.1722604821.git.fweimer@redhat.com
State New
Headers show
Series Turn on -Wimplicit-fallthrough | expand

Commit Message

Florian Weimer Aug. 2, 2024, 1:22 p.m. UTC 
---
 Makeconfig     |  3 +++
 config.make.in |  1 +
 configure      | 12 ++++++++++++
 configure.ac   | 10 ++++++++++
 4 files changed, 26 insertions(+)

Comments

Adhemerval Zanella Netto Aug. 7, 2024, 12:10 p.m. UTC | #1 
On 02/08/24 10:22, Florian Weimer wrote:
> ---
>  Makeconfig     |  3 +++
>  config.make.in |  1 +
>  configure      | 12 ++++++++++++
>  configure.ac   | 10 ++++++++++
>  4 files changed, 26 insertions(+)
> 
> diff --git a/Makeconfig b/Makeconfig
> index 2d4343b604..e90f612b96 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -889,6 +889,9 @@ endif
>  
>  # Extra flags to pass to GCC.
>  +gccwarn := -Wall -Wwrite-strings -Wundef
> +ifeq ($(enable-wimplicit-fallthrough),yes)
> ++gccwarn += -Wimplicit-fallthrough
> +endif
>  ifeq ($(enable-werror),yes)
>  +gccwarn += -Werror
>  endif
> diff --git a/config.make.in b/config.make.in
> index 36096881b7..4286a63269 100644
> --- a/config.make.in
> +++ b/config.make.in
> @@ -51,6 +51,7 @@ c++-cstdlib-header = @CXX_CSTDLIB_HEADER@
>  c++-cmath-header = @CXX_CMATH_HEADER@
>  c++-bits-std_abs-h = @CXX_BITS_STD_ABS_H@
>  enable-werror = @enable_werror@
> +enable-wimplicit-fallthrough = @enable_wimplicit_fallthrough@
>  
>  have-z-execstack = @libc_cv_z_execstack@
>  have-protected-data = @libc_cv_protected_data@
> diff --git a/configure b/configure
> index 1d543548cd..01a011d73f 100755
> --- a/configure
> +++ b/configure
> @@ -643,6 +643,7 @@ have_selinux
>  have_libcap
>  have_libaudit
>  LIBGD
> +enable_wimplicit_fallthrough
>  libc_cv_cc_loop_to_function
>  libc_cv_cc_submachine
>  libc_cv_cc_nofma
> @@ -7465,6 +7466,17 @@ if test $libc_cv_cc_loop_to_function = yes; then
>  fi
>  
>  
> +if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
> +   || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
> +then
> +   echo HOHOHO
> +   enable_wimplicit_fallthrough=yes
> +else
> +   echo BADBADBAD
> +   enable_wimplicit_fallthrough=no
> +fi
> +
> +
>  { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for libgd" >&5
>  printf %s "checking for libgd... " >&6; }
>  if test "$with_gd" != "no"; then
> diff --git a/configure.ac b/configure.ac
> index 9cbc0bf68f..c686ea4b97 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1468,6 +1468,16 @@ if test $libc_cv_cc_loop_to_function = yes; then
>  fi
>  AC_SUBST(libc_cv_cc_loop_to_function)
>  
> +dnl GCC uses --help=warnings, Clang uses -print-diagnostic-options.
> +if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
> +   || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
> +then
> +   enable_wimplicit_fallthrough=yes
> +else
> +   enable_wimplicit_fallthrough=no
> +fi
> +AC_SUBST(enable_wimplicit_fallthrough)
> +
>  dnl Check whether we have the gd library available.
>  AC_MSG_CHECKING(for libgd)
>  if test "$with_gd" != "no"; then

Why not AC_CACHE_CHECK and LIBC_CONFIG_VAR:

AC_CACHE_CHECK([for implicit fallthrough warning],
               libc_cv_implicit_fallthrough_warning,
libc_cv_implicit_fallthrough_warning=no
if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
   || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
then
   libc_cv_implicit_fallthrough_warning=yes
fi)
LIBC_CONFIG_VAR([enable-wimplicit-fallthrough],
                [$libc_cv_implicit_fallthrough_warning])

?
Andreas Schwab Aug. 7, 2024, 12:28 p.m. UTC | #2 
On Aug 07 2024, Adhemerval Zanella Netto wrote:

> AC_CACHE_CHECK([for implicit fallthrough warning],
>                libc_cv_implicit_fallthrough_warning,
> libc_cv_implicit_fallthrough_warning=no
> if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
>    || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"

This should check directly whether $CC accepts the option.
Adhemerval Zanella Netto Aug. 7, 2024, 12:38 p.m. UTC | #3 
On 07/08/24 09:28, Andreas Schwab wrote:
> On Aug 07 2024, Adhemerval Zanella Netto wrote:
> 
>> AC_CACHE_CHECK([for implicit fallthrough warning],
>>                libc_cv_implicit_fallthrough_warning,
>> libc_cv_implicit_fallthrough_warning=no
>> if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
>>    || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
> 
> This should check directly whether $CC accepts the option.
> 

Why exactly we can't use AC_CACHE_CHECK in this case?
Florian Weimer Aug. 7, 2024, 12:56 p.m. UTC | #4 
* Adhemerval Zanella Netto:

> On 07/08/24 09:28, Andreas Schwab wrote:
>> On Aug 07 2024, Adhemerval Zanella Netto wrote:
>> 
>>> AC_CACHE_CHECK([for implicit fallthrough warning],
>>>                libc_cv_implicit_fallthrough_warning,
>>> libc_cv_implicit_fallthrough_warning=no
>>> if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
>>>    || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
>> 
>> This should check directly whether $CC accepts the option.
>> 
>
> Why exactly we can't use AC_CACHE_CHECK in this case?

I think Andreas just wants to check the option directly (clang needs
-Werror, didn't figure that out before), rather than searching for it in
help output.  I'll see if I can produce a patch along those lines.

Thanks,
Florian
diff mbox series

Patch

diff --git a/Makeconfig b/Makeconfig
index 2d4343b604..e90f612b96 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -889,6 +889,9 @@  endif
 
 # Extra flags to pass to GCC.
 +gccwarn := -Wall -Wwrite-strings -Wundef
+ifeq ($(enable-wimplicit-fallthrough),yes)
++gccwarn += -Wimplicit-fallthrough
+endif
 ifeq ($(enable-werror),yes)
 +gccwarn += -Werror
 endif
diff --git a/config.make.in b/config.make.in
index 36096881b7..4286a63269 100644
--- a/config.make.in
+++ b/config.make.in
@@ -51,6 +51,7 @@  c++-cstdlib-header = @CXX_CSTDLIB_HEADER@
 c++-cmath-header = @CXX_CMATH_HEADER@
 c++-bits-std_abs-h = @CXX_BITS_STD_ABS_H@
 enable-werror = @enable_werror@
+enable-wimplicit-fallthrough = @enable_wimplicit_fallthrough@
 
 have-z-execstack = @libc_cv_z_execstack@
 have-protected-data = @libc_cv_protected_data@
diff --git a/configure b/configure
index 1d543548cd..01a011d73f 100755
--- a/configure
+++ b/configure
@@ -643,6 +643,7 @@  have_selinux
 have_libcap
 have_libaudit
 LIBGD
+enable_wimplicit_fallthrough
 libc_cv_cc_loop_to_function
 libc_cv_cc_submachine
 libc_cv_cc_nofma
@@ -7465,6 +7466,17 @@  if test $libc_cv_cc_loop_to_function = yes; then
 fi
 
 
+if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
+   || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
+then
+   echo HOHOHO
+   enable_wimplicit_fallthrough=yes
+else
+   echo BADBADBAD
+   enable_wimplicit_fallthrough=no
+fi
+
+
 { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for libgd" >&5
 printf %s "checking for libgd... " >&6; }
 if test "$with_gd" != "no"; then
diff --git a/configure.ac b/configure.ac
index 9cbc0bf68f..c686ea4b97 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1468,6 +1468,16 @@  if test $libc_cv_cc_loop_to_function = yes; then
 fi
 AC_SUBST(libc_cv_cc_loop_to_function)
 
+dnl GCC uses --help=warnings, Clang uses -print-diagnostic-options.
+if ${CC-cc} --help=warnings 2>&1 | grep -q " -Wimplicit-fallthrough" \
+   || ${CC-cc} -print-diagnostic-options 2>&1 | grep -q " -Wimplicit-fallthrough"
+then
+   enable_wimplicit_fallthrough=yes
+else
+   enable_wimplicit_fallthrough=no
+fi
+AC_SUBST(enable_wimplicit_fallthrough)
+
 dnl Check whether we have the gd library available.
 AC_MSG_CHECKING(for libgd)
 if test "$with_gd" != "no"; then