nholthaus / units

a compile-time, header-only, dimensional analysis and unit conversion library built on c++14 with no dependencies.
http://nholthaus.github.io/units/
MIT License
934 stars 134 forks source link

Incorrect enable_if condition for operator+ #299

Open BenjaminNavarro opened 1 year ago

BenjaminNavarro commented 1 year ago

I'm currently using v2.3.1 but after looking at the current version, the problem is still there.

operator+ is defined like:

template<class UnitTypeLhs, class UnitTypeRhs, std::enable_if_t<!traits::is_same_scale<UnitTypeLhs, UnitTypeRhs>::value, int> = 0>
constexpr inline int operator+(const UnitTypeLhs& /* lhs */, const UnitTypeRhs& /* rhs */) noexcept
{
    static_assert(traits::is_same_scale<UnitTypeLhs, UnitTypeRhs>::value, "Cannot add units with different linear/non-linear scales.");
    return 0;
}

template<class UnitTypeLhs, class UnitTypeRhs, std::enable_if_t<traits::has_linear_scale<UnitTypeLhs, UnitTypeRhs>::value, int> = 0>
inline constexpr UnitTypeLhs operator+(const UnitTypeLhs& lhs, const UnitTypeRhs& rhs) noexcept
{
    // actual code
}

// other operator+ implementations...

The problem I have is when defining the arithmetic operators on my custom types so that they can be used with this library easily. Something like:

using namespace units::literals;

my::Position p1, p2;
p2 = p1 + 1_m;

Using 1_m makes the compiler look for operators in the units namespace but since my::Position is not a units type, the first failsafe operator above is selected in the overload set (traits::is_same_scale<UnitTypeLhs, UnitTypeRhs>::value is false) and the static_assert is triggered.

I think the failsafe operator must also include a traits::is_unit_t<UnitTypeLhs>::value and traits::is_unit_t<UnitTypeRhs>::value in his enable_if so that it is only selected when appropriate.

I'm willing to write a PR if you think the approach is correct.

EDIT: I though this problem was for all operators but it looks like it's just operator+.

BenjaminNavarro commented 1 year ago

In fact the problem is triggered only when a custom type inherits its operator+ because in this case the compiler first look for free functions before looking into parent types.