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

Added std::common_type<T, T> specialization to ensure the return type… #314

Closed
wants to merge 4 commits into from

Conversation

Guillaume227
Copy link

Added std::common_type<T, T> specialization to ensure the return type off arithmetic operators matches the input type when the two inputs have the same type. That fixes ternary operator use cases. See #313

… of arithmetic operators matches the input type when the two inputs have the same type. That fixes ternary operator use cases.
@JohelEGP
Copy link
Contributor

JohelEGP commented Mar 31, 2023

Returning the same type as the template parameters would regress returning the GCD of the conversion factors, like https://en.cppreference.com/w/cpp/chrono/duration/common_type.

For example, let D be std::duration<int, std::ratio<2, 2>>. std::common_type_t<D, D> should be std::duration<int> (with period std::ratio<1>), not D.

# Conflicts:
#	include/units/core.h
#	unitTests/main.cpp
… of arithmetic operators matches the input type when the two inputs have the same type. That fixes ternary operator use cases.
@Guillaume227
Copy link
Author

Guillaume227 commented Apr 4, 2023

Ok thanks for raising this subtlety, which is not covered by the current test suite.
I am going to need to think about how to factor that in.
For reference, here is the error I am trying to fix when using the ternary operator:
auto deg = true ? degrees<double>(1) : degrees<double>(2) - degrees<double>(90);

/home/ggiraud/mcaura/units/unitTests/main.cpp:657:25: error: operands to ‘?:’ have different types 
‘units::angle::degrees<double>’ {aka ‘units::unit<units::angle::degrees_, double, units::linear_scale>’} and 
‘std::common_type_t<units::unit<units::angle::degrees_, double, units::linear_scale>, units::unit<units::angle::degrees_, double, 
units::linear_scale> >’ {aka ‘units::unit<units::conversion_factor<std::ratio<1, 180>, 
units::dimension_t<units::dim<units::dimension::angle_tag, std::ratio<1, 1> > >, std::ratio<1>, std::ratio<0> >, double, 
units::linear_scale>’}

@nholthaus
Copy link
Owner

note to self: I think this can be done, need to get the GCD of the conversion factors and add a test case that we're doing it right similar to Johel's

@nholthaus
Copy link
Owner

nholthaus commented Dec 17, 2024

added with tests to ensure gcd

@nholthaus nholthaus closed this Dec 17, 2024
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

Successfully merging this pull request may close these issues.

3 participants