Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect enable_if condition for operator+ #299

Open
BenjaminNavarro opened this issue Nov 18, 2022 · 1 comment
Open

Incorrect enable_if condition for operator+ #299

BenjaminNavarro opened this issue Nov 18, 2022 · 1 comment

Comments

@BenjaminNavarro
Copy link

BenjaminNavarro commented Nov 18, 2022

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 BenjaminNavarro changed the title Incorrect SFNIAE for operators with static_asserts Incorrect enable_if condition for operator+ Nov 18, 2022
@BenjaminNavarro
Copy link
Author

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.

BenjaminNavarro added a commit to BenjaminNavarro/units that referenced this issue Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant