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

multiply operations on scalars should be constrained #58

Open
kwikius opened this issue Jul 2, 2020 · 3 comments
Open

multiply operations on scalars should be constrained #58

kwikius opened this issue Jul 2, 2020 · 3 comments

Comments

@kwikius
Copy link

kwikius commented Jul 2, 2020

re compatability with units library ( and indeed other libraries)

https://github.com/mpusz/units

Both units library and la library should constrain operators multiplying by scalars to avoid ambiguous overloads.
LA library should define a scalar concept or use SFINAE version for legacy. Scalar is mentioned barely in the documentation but is an important concept for LA library

See example of the current situation and how to fix

https://godbolt.org/z/K-rWPL

( uncomment one of CONCEPT_CONSTRAINED_S, SFINAE_CONSTRAINED_S ) to get it to work

Below are ( some of) relevant functions

https://github.com/BobSteagall/wg21/blob/master/include/linear_algebra/arithmetic_operators.hpp#L104
https://github.com/BobSteagall/wg21/blob/master/include/linear_algebra/arithmetic_operators.hpp#L116
https://github.com/BobSteagall/wg21/blob/master/include/linear_algebra/arithmetic_operators.hpp#L130
https://github.com/BobSteagall/wg21/blob/master/include/linear_algebra/arithmetic_operators.hpp#L142

(Also incidentally applies to units library) ....

units uses concept but units::Scalar concept is basically Any but quantity. Not really constrained so ambiguous
https://github.com/mpusz/units/blob/master/src/include/units/quantity.h#L325
https://github.com/mpusz/units/blob/master/src/include/units/concepts.h#L292

Currently standard library doesnt have unconstrained binary operators as far as I know

@mhoemmen
Copy link

mhoemmen commented Jul 4, 2020

std::is_arithmetic is probably more restrictive than one might like, considering that one can't add specializations. I'm guessing that was just a proxy for the actual concept you want, though. This reminds me a bit of P1813, which aims to introduce concepts for algorithms like std::reduce.

@kwikius
Copy link
Author

kwikius commented Jul 5, 2020

std::is_arithmetic is probably more restrictive than one might like, considering that one can't add specializations. I'm guessing that was just a proxy for the actual concept you want, though. This reminds me a bit of P1813, which aims to introduce concepts for algorithms like std::reduce.

Absolutely std::is_arithmetic is too restrictive, but maybe is a good default. For customisation I think one can do:

template <typename T> 
inline constexpr bool is_scalar_impl = std::is_arithmetic_v<T>;

template <typename T>
concept scalar = is_scalar_impl<T>; 

// ##########customisation via the constant  #############
template <typename T>
inline constexpr bool is_scalar_impl<my_quantity<T> > = true;
//##########################

P1813 is very interesting. There is a debate to be had about using "structural" ( quacks like a duck) concepts rather than opting your type in manually , as above.

@mhoemmen
Copy link

mhoemmen commented Jul 5, 2020

Btw I'm not entirely advocating P1813 as it stands. For instance, I don't think it's right to constrain std::reduce to types with associative operator+. Many users are OK with floating-point arithmetic (or other systems, e.g., saturating integers) being nonassociative. GENERALIZED_SUM exists to tell you what std::reduce does, so that you can evaluate whether to accept its results.

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

2 participants