-
Notifications
You must be signed in to change notification settings - Fork 104
Description
See https://godbolt.org/z/sn1eWTd4Y for example.
Of course, this problem is broadly known, which is why std::cmp_less
and friends were added to C++20. As mp-units mostly just wraps the underlying raw types, we'd expect to have this problem here too. However, it's actually worse in this case, because the implementation of operator<=>
explicitly casts to the common type, and this explicit cast will silence any possible warnings from -Wsign-compare
.
OK, so we can be a little more careful in what we cast to before comparing, right? Well, even then we're not done, because it turns out that many (most?) compiler settings will suppress warnings that originate inside of library code. This means that even for situations where -1 < 1u
will reliably raise a warning, (-1 * m) < (1u * m)
will often not raise the warning!
Au just found and fixed this error (aurora-opensource/au#428), so we know exactly the steps that need to happen. We used 4 PRs, but not all of them would be relevant for mp-units. I think these are the important PRs/steps.
- Avoid changing signedness of integral rep aurora-opensource/au#433: be careful what rep you use for the LHS and RHS. If the common rep is
std::integral
, then use the common rep for each, except avoid changing the signedness. - Fix comparisons for mixed-signedness integral reps aurora-opensource/au#434: if the common rep is integral, perform a careful comparison that takes this into account: kind of like "the
operator<=>
version ofstd::cmp_less
". (mp-units' C++20 minimum version should enable you to do this more elegantly than we were able to.)