Turn _arb_set and _acb_set into set! methods #1909
Conversation
Also replace some `for ... @eval begin` macro stuff Also for the former `_acb_set` methods, we now take a tuple of two integers/reals/etc. coordinates instead of two arguments to avoid ambiguities between them precision argument.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1909 +/- ##
==========================================
- Coverage 87.24% 86.85% -0.39%
==========================================
Files 97 97
Lines 35590 35585 -5
==========================================
- Hits 31049 30908 -141
- Misses 4541 4677 +136 ☔ View full report in Codecov by Sentry. |
| _real_ptr(x::ComplexFieldElemOrPtr) = @ccall libflint.acb_real_ptr(x::Ref{ComplexFieldElem})::Ptr{RealFieldElem} | ||
| _imag_ptr(x::ComplexFieldElemOrPtr) = @ccall libflint.acb_imag_ptr(x::Ref{ComplexFieldElem})::Ptr{RealFieldElem} |
There was a problem hiding this comment.
@lgoettgens I think this and friends is something to preserve (feel free to do so if you have time, otherwise I'll get to it eventually)
... is the comment I typed an hour ago, then I got visitors and was distracted, and now I discovered you read my mind and did PR #1921 😂
| @@ -1616,135 +1616,127 @@ end | |||
| # | |||
| ################################################################################ | |||
|
|
|||
| for (typeofx, passtoc) in ((ComplexFieldElem, Ref{ComplexFieldElem}), (Ptr{ComplexFieldElem}, Ptr{ComplexFieldElem})) | |||
There was a problem hiding this comment.
Getting rid of this loop by replacing $typeofx with ComplexFieldElemOrPtr and $passtox with Ref{ComplexFieldElem} is also something we could re-apply (and similar in the other files, of course)
There was a problem hiding this comment.
agreed. I'll try to do this with as less conflicts as possible
| i = ccall((:acb_imag_ptr, libflint), Ptr{RealFieldElem}, (($passtoc), ), x) | ||
| _arb_set(i, z, p) | ||
| end | ||
| function set!(x::ComplexFieldElemOrPtr, y::Tuple{Int, Int}, p::Int) |
There was a problem hiding this comment.
One more change here that I think we should consider to retain for _acb_set methods: pairing "real+imag" arguments together into a single 2-tuple argument. That avoids ambiguities with those p::Int precision arguments...
|
I think all the good bits of this have been extracted by now. |
Also replace some
for ... @eval beginmacro stuffAlso for the former
_acb_setmethods, we now take a tuple of two integers/reals/etc. coordinates instead of two arguments to avoid ambiguities between them precision argument.This is just a draft because I later realized that these methods, or at least the ones which set a
ArbFieldElem/AcbFieldElemand take a precision valuep, should also adjust the parent (even if it is just emptying it). Or perhaps I just rename the methods back for those rings, to not worry about this. Dunno.Putting this here anyway to avoid duplicated efforts (if someone else would like to continue or supersede this work, be my guest)