Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #342 +/- ##
==========================================
- Coverage 89.86% 86.86% -3.00%
==========================================
Files 22 22
Lines 2141 2208 +67
==========================================
- Hits 1924 1918 -6
- Misses 217 290 +73 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Yes, I would welcome the addition, the macro makes implementing them quite compact! I would deal with |
- Therefore, remove special handling - the types already include Int and UInt - Add floating point type - Enhance macro: allow for pre- and postprocessing as well as custom parameter names
…versions, consistent with Base in newer ones)
|
@projekter FYI this PR fails CI:
|
|
I'm very hesitant to accept this. I have almost always come to regret using macros for code generation within a package. This is a large increase in code for what is already a very complicated package. @make_mpfr copy(::BigFloat) -> mpfr_set
# instead of
promote_operation(::typeof(copy), ::Type{BigFloat}) = BigFloat
function operate_to!(out::BigFloat, ::typeof(copy), x::BigFloat)
@ccall libmpfr.mpfr_set(
out::Ref{BigFloat},
x::Ref{BigFloat},
Base.MPFR.ROUNDING_MODE[]::Base.MPFR.MPFRRoundingMode,
)::Int32
return out
endI tend to like the more verbose code. It's much easier to reason about, and it shows the true cost of maintenance. It's also slightly tangential to our original motivation, which was a package to make JuMP's macros faster. @blegat and I need to discuss. |
I recently saw that MutableArithmetics does not support most of the functions that MPFR offers, only the basic arithmetic and assignment stuff. Actually, I needed it for assignment of (bit) integer values, but when I looked at it, I thought that just implementing the whole bunch of functions is not much more effort once you have a nice macro to automate it.
So in this PR:
However:
SpecialFunctions.jland implement the functions from there that are part of MPFR.If you want this to proceed, I think tests should be part of this PR, but the other two points could also be separate...