How to deal with implicit type conversion warnings?

[arandomhuman]

TL;DR., should they be: completely ignored, selectively fixed, or completely fixed?

This was prompted by @Laaas finding a legitimate bug because of an implicit real->int conversion (afair). I've dumped some details and potential changes into a gist.

Some stats: the master branch currently throws ~825 warnings, which are divided up as,

• -Wsign-conversion (int <-> uint): ~475;
• width downgrade (intX_t <-> intY_t): ~175;
• -Wfloat-conversion (real->int, float<->double): ~175.

---

wlroots has migrated to gitlab.freedesktop.org. This issue has been moved to:

https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/1251

[ddevault]

completely fixed

[arandomhuman]

The how still needs to be worked out, especially in the integral<->integral cases, where the effort to benefit ratio is murkier, and which make up most of the code. If there is explicit type casting, should it be with or without validity assertions? The validity assertions should be present always, or only when it is not obvious from context?

For real->integral conversions, should using floor/ceil be enforced instead of implicit integral value? Should the float value be checked for validity (nan, infinity, out of range of destination type) since that leads to undefined behaviour, but adds lots of code?

(Some examples in gist)

[ddevault]

validity assertions

What are these?

For real->integral conversions, should using floor/ceil be inforced instead of implicit integral value?

Casting is good enough for me

Should the float value be checked for validity

This one isn't tearing me up as much, unless we know that these cases are possible we needn't bother.

In most of these cases it'll be a judgement call on the part of whoever does it.

[arandomhuman]

What are these?

See the gist in the top post.

Casting is good enough for me

Tbf that's the reason for the motivating bug.

I also stumbled upon UndefinedBehaviourSanitizer, which catches most of these problems at runtime. In particular, the checks: float-cast-overflow, signed-integer-overflow, unsigned-integer-overflow, implicit-integer-truncation (introduced in clang-7.0 are interesting.

It doesn't catch assignments of negative ints -> unsigned ints, I think, so it isn't a fix per se. Useful anyway; caught an unsigned-integer-overflow in wlr_primary_selection.

unless we know that these cases are possible we needn't bother.

Update: UBSan reliably catches double->float precision errors, and real->integral irrepresentable values (finite value out of bounds, nan/(-)inf), even with explicit type casting, so we needn't check for them.

It doesn't catch integral width change and unsigned->signed value changes; if someone has clang-7.0, they could try out the implicit-integer-truncation option.