Bug 1577236 Comment 13 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Coincidentally I discovered this warning when I happened to need to rebuild my tip clang yesterday.  I took care of one of the MFBT warning in bug 1577051 and one of the JS warnings in bug 1577065.  We're hashing out a proper fix for the other JS warning in bug 1577066.

While merely making the conversions explicit is enough to silence the warning, it's not always the best, most ideal fix IMO.  Inexact int-to-float conversions have only implementation-defined behavior, which is a bit more obfuscatory than writing code that _doesn't_ depend on such.  And in likely an awful lot of these cases, what is being tested is overflow conditions (because `FOO_MAX` is what's being cast).  It often is going to be better to convert `flt <= FOO_MAX` to `flt <= float(FOO_MAX + 1 carefully calculated to not immediately overflow)`, which is what the JS changes ended up doing.  Reviewers probably should consider similar sorts of changes to just spraying explicit conversions everywhere.
Coincidentally I discovered this warning when I happened to need to rebuild my tip clang yesterday.  I took care of one of the MFBT warning in bug 1577051 and one of the JS warnings in bug 1577065.  We're hashing out a proper fix for the other JS warning in bug 1577066.

While merely making the conversions explicit is enough to silence the warning, it's not always the best, most ideal fix IMO.  Inexact int-to-float conversions have only implementation-defined behavior, which is a bit more obfuscatory than writing code that _doesn't_ depend on such.  And in likely an awful lot of these cases, what is being tested is overflow conditions (because `FOO_MAX` is what's being cast).  It often is going to be better to convert `flt <= FOO_MAX` to `flt < float(FOO_MAX + 1 carefully calculated to not immediately overflow)`, which is what the JS changes ended up doing.  Reviewers probably should consider similar sorts of changes to just spraying explicit conversions everywhere.

Back to Bug 1577236 Comment 13