`operator==` can lead to UB

[JohelEGP]

The list I'm making was getting long, so I decided to break off some issues that are better off by themselves.

A recent r/cpp post https://www.reddit.com/r/cpp/comments/8gfhq3/when_the_compiler_bites/ touches on some points about floating-point arithmetic and comparisons, which leads me to believe that the current implementation of operator== for floating point values, which uses epsilon values, should be reimplemented to be as simple as the operator== for integral values. As it is, it can make using std::set<units::meter_t> UB because it breaks the transitivity requirement of the Compare argument.

[nholthaus]

You're not wrong, but this falls into the "it works the way I want it to" category.

Many conversions are not transitive (e.g. dBW -> W -> dBW), and a lot of trig identities are broken (sin(pi) == 0) without using an epsilon compare. So you're forced to choose between supporting the identity, or transitivity, and my opinion is that it's more important for usability to support identity. std::set<double> may be well-defined, but honestly would you wouldn't do this:

std::set<double> s;
s.insert(0.0);
//...
double someDouble = ...; // something that could/should be 0
if(!s.count(someDouble))
{
    // probably not going to work as expected...
}

Additionally, the epsilon for equality isn't arbitrary, it's well chosen to suite the underlying type.

There are arbitrary epsilons in the code though (look into the unit sqrt calculations). I don't like it, but it's a better alternative than having most conversions fail from recursive depth limits or integer overflow.

[JohelEGP]

Very well. Might I suggest documenting the use of epsilon in the comparisons?

[chiphogg]

I came here to file this exact issue after getting bitten by this behaviour. Can we reconsider the wontfix label?

It's well and good to have an "approximately-equal" function, but we should give it a different name. If operator== doesn't create equivalence classes, that's a pretty significant violation of users' expectations. It also leads to undefined behaviour (!), as @JohelEGP has pointed out.

Users who work with floating point values should know not to compare the results of arithmetic using exact equality: this is one of the most basic rules for working with floating point values. As to some of the specific examples given above:

• It's perfectly reasonable for sin(pi) to be different from 0.0, because pi is not exactly equal to $\pi$; instead, it's the nearest number we can represent in some floating point type.

• I also think the set<double> example is an intrinsically error-prone construct, and I would be suspicious of code that uses it. What's more, the approximate equality does not fix it. std::set::count considers two elements equivalent if a < b and b < a are both false. Since the library does not use an epsilon here, this will not be the case, and the almost-equal-to-0 value will still not be found. (To be clear: this is the correct behaviour; my point is that it's not an argument in favor of incorporating an epsilon into ==.)

To sum up: I think the operator== you've provided is a really useful tool---I love how it takes care of a reasonable epsilon for the user! I just think it's important not to give it the operator== name.