From: Fangrui Song <i@maskray.me>
To: Rich Felker <dalias@libc.org>
Cc: musl@lists.openwall.com
Subject: Re: [PATCH 1/5] fix warning dangling-else
Date: Tue, 23 Jul 2019 04:06:31 +0000 [thread overview]
Message-ID: <20190723040631.a7ug6glq62hkfpzz@gmail.com> (raw)
In-Reply-To: <20190723033807.GA15665@brightrain.aerifal.cx>
On 2019-07-22, Rich Felker wrote:
>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.
The annoying group -Wparentheses is enabled by default in clang.
-Wdangling-else is within the group. I incorrectly thought it is
desired (in my own projects I don't like these warnings, but oftentimes I
just submit to the default warning rule..)
If -Wmisleading-indentation (not supported by clang yet) captured the
following case, I would agree it is strictly better than
-Wdangling-else:
if (...)
if (...)
;
else
;
--- c/configure
+++ w/configure
@@ -517 +516,0 @@ tryflag CFLAGS_AUTO -Wall
-tryflag CFLAGS_AUTO -Wno-parentheses
@@ -524,0 +524,2 @@ fi
+tryflag CFLAGS_AUTO -Wno-string-plus-int
+tryflag CFLAGS_AUTO -Wno-parentheses
Some clang packages may ship the tool "diagtool", diagtool tree gives a
hierarchy of warnings/warning groups. The green ones mark "enabled-by-default warnings.
next prev parent reply other threads:[~2019-07-23 4:06 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-22 18:07 Issam Maghni
2019-07-22 18:07 ` [PATCH 2/5] fix warning logical-op-parentheses Issam Maghni
2019-07-22 18:07 ` [PATCH 3/5] fix warning bitwise-op-parentheses Issam Maghni
2019-07-22 18:07 ` [PATCH 4/5] fix warning shift-op-parentheses Issam Maghni
2019-07-22 18:07 ` [PATCH 5/5] fix warning parentheses Issam Maghni
2019-07-22 18:50 ` [PATCH 1/5] fix warning dangling-else A. Wilcox
2019-07-22 20:50 ` Rich Felker
2019-07-23 2:31 ` Fangrui Song
2019-07-23 3:38 ` Rich Felker
2019-07-23 4:06 ` Fangrui Song [this message]
2019-07-23 4:25 ` Rich Felker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190723040631.a7ug6glq62hkfpzz@gmail.com \
--to=i@maskray.me \
--cc=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).