-
Notifications
You must be signed in to change notification settings - Fork 137
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
v2 -> v3 porting #313
Comments
Wait, it seems I was missing some significant updates to v3.x, for example the _t suffix is gone entirely now. I need to review and update what I said above. |
Ok after a bit of renaming I managed to make almost everything work on 3.x - I updated all the PRs above, they should be good for review/feedback. I also opened #318 and #317, they are self-explanatory Overall things feel much cleaner in 3.x, Thanks. Straightening up the ADL story makes a big difference. Regarding testing, my use case makes use of |
I have been attempting to convert a codebase using version 2 to the 3.x branch and here is some feedback and questions.
I embarked on this upgrade because I grew tired of the inconsistent
to_string
vsstream
behavior for units like volt_t.My initial, naive, expectation was a drop-in replacement with no change needed in the rest of the code base.
Here is what happened in the hope that's useful for people going through an update. I am planning to send a few PRs as follow-up.
Most of the issues stemmed from using a
float
underlying type - all of the unit testing is done fordouble
.I used to do:
In 3.x I do:
using meter_t = units::length::meter_t<float>;
In 3..x it a bit less nicely packaged when it comes to literal operators since I am dealing with floats.
I have to redefine them outside of core.h with a user-land macro since I do not want
double
:It's not too bad though, I just had to reintroduce
UNIT_HAS_LITERAL_SUPPORT
in 3.x.I tracked it down to a 0.5 literal that was causing a mixture of double and floats in the call to
sqrtNewtonRaphson
. Once found, it was straightforward to replace that with T{0.5}.I hit an issue where the ternary operator no longer compiles:
After sifting through the template compilation errors, it's down to:
std::common_type_t<degree_t, degree_t>
differing fromdegree_t
, as it's using a dummy 1 conversion factor.That can be solved through another
std::common_type_t
specialization.The text was updated successfully, but these errors were encountered: