Clang emits -Wfloat-equal warning in consumer projects

[mxgrey]

I'm using clang-6.0 on Ubuntu 16.04, and I've found that projects that consume dartsim version 6 will get hit by the -Wfloat-equal warning from GenericJoint.hpp if they try to instantiate any joint types.

The consumer projects can avoid this by adding the dartsim headers as SYSTEM headers, but if it's possible to do a bitwise comparison of floating point variables without triggering this warning, then it would be good to switch to that.

The only way I can think of doing that off the top of my header is something goofy like:

reinterpret_cast<long long&>(value) == reinterpret_cast<long long&>(Base::mAspectProperties.mField)

but I'd prefer to avoid solutions that involve reinterpret_cast or make assumptions about the sizes of primitive types.

[jslee02]

Could you let me know which line is it in GenericJoint.hpp?

[mxgrey]

It's this line in detail/GenericJoint.hpp, where we compare the user-requested value against the existing value.

It's a rare case where we really do want bit-wise comparison of two floats.

[jslee02]

If we just want to suppress that warnings, then we could just disable it using this approach (edit: fixed the link).

I think the next question is that whether we really want to do bit-wise comparison for float point values, which I don't have a preference right now. 🤔

[mxgrey]

We absolutely want bitwise comparison for these floating point values, because otherwise we would inadvertently filter out small changes in input values. The purpose of the comparison is to avoid re-triggering a cascade of updates when the user passes in an input value that definitely wouldn't have any effect. But if the user passes in a value that does entail any difference, we want to let it through, no matter how quantitatively small that difference might be.

[jslee02]

I agree. Let's then perform the bitwise comparison. FYI, I found a SO post about size agnostic bitwise comparison.

[mxgrey]

I think I would much rather use warning suppression macros than do anything that involves reinterpret_cast.

[jslee02]

Does that mean == operates exactly same as the bitwise comparison? If so, I would also prefer warning suppression approach!

[mxgrey]

Yes, the equality operation should be the kind of comparison we want. The post you linked seems to be talking about comparing specific bits within the floating point representation, but we only really care about comparing the exact values as a whole.

Edit: Also, the value equality comparison would be better for us since +0.0 and -0.0 should compare the same, but they wouldn't with a strict bitwise comparison. There might be some other edge cases that a value equality comparison would take care of.

[jslee02]

Addressed by #2225