-
Notifications
You must be signed in to change notification settings - Fork 8
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
copy
for BigInt
, BigFloat
and Rational
#238
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #238 +/- ##
==========================================
+ Coverage 89.66% 89.72% +0.06%
==========================================
Files 23 23
Lines 2157 2171 +14
==========================================
+ Hits 1934 1948 +14
Misses 223 223 ☔ View full report in Codecov by Sentry. |
The failing Aqua lint is unrelated (Aqua is complaining about missing compat bounds for dependencies). |
I can fix this; #239 |
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.
Looks okay to me, but this needs @blegat's review.
) | ||
return nothing | ||
end | ||
set! = (o, i) -> operate_to!(o, copy, i) |
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.
I think you need to keep the local
to avoid the performance issue of closures
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.
No, it's fine:
julia> import MutableArithmetics, LinearAlgebra; const MA = MutableArithmetics;
julia> x = BigFloat[1, 3];
julia> y = BigFloat[3, 1];
julia> buf = MA.buffer_for(LinearAlgebra.dot, typeof(x), typeof(y))
MutableArithmetics.DotBuffer{BigFloat}(NaN, NaN, NaN, NaN)
julia> import MutableArithmetics, LinearAlgebra; const MA = MutableArithmetics;
julia> x = BigFloat[1, 3];
julia> y = BigFloat[3, 1];
julia> buf = MA.buffer_for(LinearAlgebra.dot, typeof(x), typeof(y));
julia> using JET
julia> @report_opt MA.buffered_operate_to!(buf, big(1.2), LinearAlgebra.dot, x, y)
No errors detected
julia> @report_call MA.buffered_operate_to!(buf, big(1.2), LinearAlgebra.dot, x, y)
No errors detected
TBH I'm not even completely sure why I used local
back when I wrote the code. I think I may have simply considered it good style to use local
with any local variable. I don't use local
at all any more.
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.
Besides, the tests would have caught any extra allocations.
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.
Thanks!
Fixes #163.