Re: [PATCH] arm64/cache: silence -Woverride-init warnings

From: Will Deacon <will@kernel.org>
To: Qian Cai <cai@lca.pw>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arm64/cache: silence -Woverride-init warnings
Date: Fri, 9 Aug 2019 10:04:13 +0100	[thread overview]
Message-ID: <20190809090413.c57d7qlqgihdyzt6@willie-the-truck> (raw)
In-Reply-To: <D2A2F2B9-0563-4DF6-8E77-F191A768CE4E@lca.pw>

On Thu, Aug 08, 2019 at 06:18:39PM -0400, Qian Cai wrote:
> > On Aug 8, 2019, at 6:38 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Please just turn this off by default for clang.
> 
> As mentioned before, it is very valuable to run “make W=1” given it found
> many real developer mistakes which will enable “-Woverride-init” for both
> compilers. Even “-Woverride-init” itself is useful find real issues as in,

Every single patch you've sent to me resulting from building with "W=1" has
been a false positive. Every. Single. One. That's not what I would call
"very valuable". It's probably what I'd call a "colossal waste of
everybody's time", especially as your tendancy to shoot from the hip when
writing these so-called fixes has resulted in some patches that ACTUALLY
INTRODUCED REAL BUGS. Do you see the insanity here?

> ae5e033d65a (“mfd: rk808: Fix RK818_IRQ_DISCHG_ILIM initializer”)
> 32df34d875bb (“[media] rc: img-ir: jvc: Remove unused no-leader timings”)
> 
> Especially, to find redundant initializations in large structures. e.g.,
> 
> e6ea0b917875 (“[media] dvb_frontend: Don't declare values twice at a table”)
> 
> It is important to keep the noise-level as low as possible by keeping the
> amount of false positives under control to be truly benefit from those
> valuable compiler warnings. 

So that's where you and I have a disagreement. I think maintainability
of the code is the single most important thing to consider after
correctness.

If code isn't maintainable, then it's liable to churn and be a constant
source of bugs as people keep tripping over whatever subtleties lie within.
In some cases, we have little choice and the combination of things like
performance requirements and compatibility force us down a path which we
wouldn't otherwise have considered. However, appeasing a compiler warning
that *doesn't even appear with the default build options* does not quality
for this sort of treatment in my opinion, so I will not be applying your
patch.

Please stop sending it. Real fixes, sure, but not this rubbish.

Will