-
Notifications
You must be signed in to change notification settings - Fork 94
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
[WIP] Bugfix: quantity_cast
fails to compile when provided with a lvalue reference
#569
[WIP] Bugfix: quantity_cast
fails to compile when provided with a lvalue reference
#569
Conversation
quantity_cast
fails to compile when provided with a lvalue referencequantity_cast
fails to compile when provided with a lvalue reference
test/static/quantity_test.cpp
Outdated
@@ -937,7 +937,11 @@ static_assert(is_of_type<quantity_cast<isq::distance>(1 * m), quantity<isq::dist | |||
static_assert(is_of_type<quantity_cast<isq::distance>(isq::length(1 * m)), quantity<isq::distance[m], int>>); | |||
static_assert(is_of_type<quantity_cast<kind_of<isq::length>>(isq::length(1 * m)), quantity<si::metre, int>>); | |||
static_assert(is_of_type<quantity_cast<kind_of<isq::length>>(isq::distance(1 * m)), quantity<si::metre, int>>); | |||
|
|||
// lvalue references in quantity_cast | |||
inline constexpr quantity<isq::distance[m], int> to_distance(quantity<m, int> arg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline
is not needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I am not sure if we need a function here as it is not a test (although it has to be compiled correctly).
Maybe it would be better to define a quantity variable and pass it to a static_assert()
. With this, you could check it the output type is exactly what you require as well.
Co-authored-by: Mateusz Pusz <mateusz.pusz@gmail.com>
This bug is really embarrassing. Thanks for finding and fixing it! |
BTW, CMake CI is broken now because the new Conan version is not compatible with the previous one. I have to fix it when I get some time. So do not be surprised by one failing GitHub action. |
Uh oh, I believe |
No description provided.