Page MenuHomeFreeBSD

Don't add -Winline for WARNS=6
ClosedPublic

Authored by arichardson on Mar 12 2021, 7:02 PM.

Details

Summary

This warning is very rarely useful (inline is a hint and not mandatory).
This flag results in many warnings being printed when compiling C++
code that uses the standard library with GCC.

This flag was originally added in back in r94332 but the flag is a no-op
in Clang ("This diagnostic flag exists for GCC compatibility, and has no
effect in Clang"). Removing it should make the GCC build output slightly
more readable.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

share/examples/etc/make.conf
97 ↗(On Diff #85669)

This one probably should stay or the entire thing rewritten

share/examples/etc/make.conf
97 ↗(On Diff #85669)

sure will drop

Looks fine to me in general.

crypto/openssh/regress/misc/kexfuzz/Makefile
57 ↗(On Diff #85669)

These are upstream files in openssh that I think we don't use in FreeBSD itself and should probably be left as-is.

tools/regression/capsicum/syscalls/Makefile
5

I wonder how stale these warning flags are and why this isn't just using bsd.prog.mk. Not a problem for this review to solve though.

crypto/openssh/regress/misc/kexfuzz/Makefile
57 ↗(On Diff #85669)

Thanks, wasn't sure about those.

This warning very rarely useful (it's a hint and not mandatory) and it

You're missing an "is", and the "it" is a bit confusing (grammatically it should refer to the warning), I'd suggest expanding to "inline is a hint".

arichardson edited the summary of this revision. (Show Details)
arichardson edited the summary of this revision. (Show Details)
arichardson edited the summary of this revision. (Show Details)

These look good to me (one I'm torn on, but I'm good either way now that I've given my feedback).
The bootloader ones I explicitly approve and thing they are right
The others look good to me.

stand/i386/boot2/Makefile
35

Unconditional elimination is fine. We wanted this in yesteryear to help us understand bloat, but it's not been that helpful lately.

stand/i386/isoboot/Makefile
32

Same as with other bootloader stuff.
I'd like to get rid of the -Wno-pointer-sign, but that's another day

sys/conf/kern.mk
51

I'm torn on this one staying or going... If someone has extra flags that include -Winline, is it a bug for the kernel? Or should the kernel compile with the warnings... I'm unsure.

This revision is now accepted and ready to land.Mar 12 2021, 8:10 PM
sys/conf/kern.mk
51

We can keep this one to prevent people from getting -Werror failures. But then again maybe we should just say "don't add -Winline to CFLAGS".

sys/conf/kern.mk
51

-Winline doesn't warn about functions in system headers, so anything out-of-tree should be entirely unaffected, with the warnings only being about _their_ code.

sys/conf/kern.mk
51

I think this one can go. Someone has to explicitly add -Winline while using GCC. If they want that, then they can deal with if they want to treat the new warning as an error or not. This list is about how we treat warnings we explicitly enable (or don't disable) in the tree.

This revision was automatically updated to reflect the committed changes.