[llvmlinux] [PATCH] sched/fair: disable clang -Wconstant-logical-operand
Lukas Bulwahn
lukas.bulwahn at gmail.com
Mon Aug 27 19:39:46 UTC 2018
On Mon, 27 Aug 2018, Peter Zijlstra wrote:
>
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> On Mon, Aug 27, 2018 at 10:18:09AM -0700, Nick Desaulniers wrote:
> > Please file a bug. There's more issues with clang in this regard:
> > https://bugs.llvm.org/show_bug.cgi?id=38571 (there should be more
> > warnings in this code). IMO, we should fix the code (if under all
> > configs the expression contains constants), not disable the warning.
>
> It's just one config that produces the warning. IMO the warning is
> pretty useless.
>
> That said; I never got a response on:
>
> https://lkml.kernel.org/r/20180420165139.GP4064@hirez.programming.kicks-ass.net
>
> which asked if:
>
> #define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
>
> would work, since that casts to a bool.
Peter, we did some further investigation since that last proposal above
and considered three options:
Option 1: Modify the code, such that the compiler does not warn about it.
IMO that would obfuscate the code just to make the compiler not warn about
that (in your words) 'dodgy compiler warning'.
Option 2: Deactivate the compiler warning globally, as you considered the
warning dodgy anyway.
Option 3: Deactivate the compiler warning locally, as there are cases
where the compiler warning does point out mistakes, but in fair.c, it is
simply a false positive.
You have seen that we proposed first a solution for option 1, but we
thought it is counterproductive to modify readable and sane code, just to
make the compiler not warn about it. So, we dropped option 1.
To decide on option 2 or 3, we searched for bugs and bug fixes in the
kernel history that the warning would have indicated. We found about ten
such cases where the warning would have been useful. Philipp can point to
the specific cases we found.
So, we decided for option 3 and hence, this patch.
Lukas
> _______________________________________________
> LLVMLinux mailing list
> LLVMLinux at lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/llvmlinux
>
More information about the LLVMLinux
mailing list