Re: [PATCH 1/5] fix warning dangling-else

[Rich Felker <dalias@libc.org>]

On Tue, Jul 23, 2019 at 02:31:24AM +0000, Fangrui Song wrote:
> With the attached patch, gcc has just some warnings in src/ctype/towctrans.c
> 
> [-Wdangling-else]
>  supposedly it will be address soon: "In the case of patch 1 here,
>  there's actually a pending replacement implementation for the whole
>  file."
> 
> clang has a few more:
> 
> % grep -o '\[-.*\]' /tmp/clang.log | sort | uniq -c
> 4 [-Wdangling-else]
> 10 [-Wignored-attributes]
>     they are all in the form of `weak_alias(statfs, statfs64)`.
>     these warnings will go away when the lfs64 things are fixed.
> 18 [-Wunknown-pragmas]
>     src/math/fmal.c:167:15: warning: pragma STDC FENV_ACCESS ON is not supported, ignoring pragma [-Wunknown-pragmas]
>             #pragma STDC FENV_ACCESS ON
>     There is a long-standing bug https://bugs.llvm.org/show_bug.cgi?id=8100
>     "[llvm-dev] [cfe-dev] Why is #pragma STDC FENV_ACCESS not supported?" was a 2018 discussion on this topic.
> 
> [-Wdangling-else] and [-Wignored-attributes] will go away soon.

> From bf24cf2d5717505b5c880d2eb6714789f86a902c Mon Sep 17 00:00:00 2001
> From: Fangrui Song <i@maskray.me>
> Date: Tue, 23 Jul 2019 02:02:47 +0000
> Subject: [PATCH] disable some known-unwanted but enabled-by-default warnings
>  in clang
> 
> the known-unwanted -Wstring-plus-int and the warning group -Wparentheses
> are enabled by default in clang. adjust CFLAGS_AUTO to disable these
> warnings whether or not --enable-warnings is specified.
> ---
>  configure | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 86801281..7f63a873 100755
> --- a/configure
> +++ b/configure
> @@ -514,7 +514,6 @@ test "$cc_family" = clang && tryflag CFLAGS_AUTO -Qunused-arguments
>  
>  if test "x$warnings" = xyes ; then
>  tryflag CFLAGS_AUTO -Wall
> -tryflag CFLAGS_AUTO -Wno-parentheses
>  tryflag CFLAGS_AUTO -Wno-uninitialized
>  tryflag CFLAGS_AUTO -Wno-missing-braces
>  tryflag CFLAGS_AUTO -Wno-unused-value
> @@ -522,6 +521,9 @@ tryflag CFLAGS_AUTO -Wno-unused-but-set-variable
>  tryflag CFLAGS_AUTO -Wno-unknown-pragmas
>  tryflag CFLAGS_AUTO -Wno-pointer-to-int-cast
>  fi
> +tryflag CFLAGS_AUTO -Wno-string-plus-int
> +tryflag CFLAGS_AUTO -Wno-parentheses
> +tryflag CFLAGS_AUTO -Wdangling-else

Why is the patch adding a test to *enable* a warning outside of the
--enable-warnings case? The -Wno's here make sense -- maybe we
should just add the disables for warnings we don't want that we know
clang or cparser have on by default, to avoid having to worry about -w
discrepancy between gcc and others.

Regarding -Wdangling-else itself, it's still a style rule that's not
followed in musl. The similar -Wmisleading-indentation seems closer to
style we do generally follow and might be appropriate under
--enable-warnings, if it doesn't have any annoying false positives.

Rich