From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14432 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH 1/5] fix warning dangling-else Date: Tue, 23 Jul 2019 00:25:17 -0400 Message-ID: <20190723042517.GA16460@brightrain.aerifal.cx> References: <20190722180740.16951-1-me@concati.me> <20190722205009.GA11895@brightrain.aerifal.cx> <20190723023124.zvfztree4fs7fonb@gmail.com> <20190723033807.GA15665@brightrain.aerifal.cx> <20190723040631.a7ug6glq62hkfpzz@gmail.com> Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="233030"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14448-gllmg-musl=m.gmane.org@lists.openwall.com Tue Jul 23 06:25:34 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1hpmMl-000yUx-LC for gllmg-musl@m.gmane.org; Tue, 23 Jul 2019 06:25:31 +0200 Original-Received: (qmail 22020 invoked by uid 550); 23 Jul 2019 04:25:29 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 22001 invoked from network); 23 Jul 2019 04:25:29 -0000 Content-Disposition: inline In-Reply-To: <20190723040631.a7ug6glq62hkfpzz@gmail.com> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14432 Archived-At: On Tue, Jul 23, 2019 at 04:06:31AM +0000, Fangrui Song wrote: > 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 > >>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..) I see, I missed that you were "undoing" part of the -Wno-parentheses. But I still would just leave it out; it's not really wanted. > 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 > ; I think it does but I'm not sure. I'm mildly worried about unfixable false positives in the case of #if tho -- things like: #if ... if (foo) ...; else #endif ...; Which are going to be needed a lot to deal with the kernel mess of omitting random sets of syscalls on each arch, in implementing the right fallback chains for time64 stuff... Rich