Don't fail the build due to clang integer constant range warnings

[arichardson]

Summary

This warning checks whether a constant is out of range of the integer
type. An example is comparison of 'u_int' > 4294967295 is always false
and in this case the warning makes sense.
However, when the type is a typedef that can be either 64 or 32 bits the
if condition is only tautological in some configurations so this should
not be a warning that fails the build.

Test Plan

Build no longer fails due to -Werror

[jhb]

Adding dim@ who normally manages updates to llvm in the base. Dimitry, Alex is building CheriBSD with a top-of-tree-ish clang. I'm not sure if you are currently building FreeBSD with clang 6 and have hit the same build failures already? Does disabling errors for this warning seem sensible to you?

[emaste]

How many warnings of this type does the build produce?

[arichardson]

I think there were about 5-8 warnings in 4 directories. https://github.com/CTSRD-CHERI/cheribsd/commit/8bd5978bcdd4d66fbc65a24a743fa571d6e40c97 is the workaround I used for CheriBSD.

[dim]

I wouldn't put it in this place, but a little higher, where the different WARNS levels are handled. For example, you might want to suppress these for WARNS <= 3. E.g., in the block:

.if ${WARNS} <= 3
CWARNFLAGS.clang+=      -Wno-tautological-compare -Wno-unused-value\
                -Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion
.if ${COMPILER_TYPE} == "clang" && ${COMPILER_VERSION} >= 30600
CWARNFLAGS.clang+=      -Wno-unused-local-typedef
.endif
.if ${COMPILER_TYPE} == "clang" && ${COMPILER_VERSION} >= 40000
CWARNFLAGS.clang+=      -Wno-address-of-packed-member
.endif
.endif # WARNS <= 3

add a new stanza for 60000 and higher.

[arichardson]

moved block to the right location

[dim]

LGTM

[arichardson]

share/mk/bsd.sys.mk

85 ↗

(On Diff #35948)

I just noticed this is slightly different. For consistency should I also use CWARNFLAGS.clang here?

[jhb]

(And you can mark approved by me now)

share/mk/bsd.sys.mk

85 ↗

(On Diff #35948)

Yes.

[arichardson]

I just realized that after moving the line further up the build fails again for me. I forgot to do a clean build after making the change...

The problem is that the .if defined(WFORMAT) block further down adds another -Werror to the command line which overrides the -Wno-error that I added. Any objections to moving that block further up? I though about moving it just below the other block that sets -Werror (line 37)

[brooks]

Isn't this OBE at this point?

[arichardson]

I believe this should probably be reverted now since the warning has been changed to off-by-default because lots of other projects also had issues with it.

[arichardson]

Fixed in rS328816

[dim]

Yeah, upstream also dropped it from the default -Wextra flags, due to too many false positives, here: https://reviews.llvm.org/rL322901. This got merged to the 6.0 branch in https://reviews.llvm.org/rL322931, which came into our tree with rS328381. I think this should be no longer necessary.