Fix conversion Irrational => Rational #16527
Conversation
simonbyrne
force-pushed
the
sb/rational-irrational
branch
from
d67919a to
43c1369
Compare
| @@ -13,7 +13,22 @@ promote_rule{s,T<:Number}(::Type{Irrational{s}}, ::Type{T}) = promote_type(Float | |||
| convert(::Type{AbstractFloat}, x::Irrational) = Float64(x) | |||
| convert(::Type{Float16}, x::Irrational) = Float16(Float32(x)) | |||
| convert{T<:Real}(::Type{Complex{T}}, x::Irrational) = convert(Complex{T}, convert(T,x)) | |||
| convert{T<:Integer}(::Type{Rational{T}}, x::Irrational) = convert(Rational{T}, Float64(x)) | |||
|
|
|||
| @generated function convert{T<:Integer}(::Type{Rational{T}}, x::Irrational) | |||
There was a problem hiding this comment.
Maybe I am mistaken but this doesn't seem to return an expression like a nvm@generated function should. It also doesn't really seem to need to be a @generated function at all.
There was a problem hiding this comment.
Returning a value is okay (it just gets spliced in as an expression).
The aim of the @generated is to get it to perform the calculation at compile time, as the result will always be constant. This is exactly the same as the usage in the function directly below.
Technically, an @pure annotation would be sufficient , once #14324 is sorted out.
There was a problem hiding this comment.
I think putting @pure on this function would just work.
There was a problem hiding this comment.
I tried, it didn't.
There was a problem hiding this comment.
simonbyrne
force-pushed
the
sb/rational-irrational
branch
from
43c1369 to
74ab9c1
Compare
| p = 256 | ||
| while true | ||
| setprecision(BigFloat, p) | ||
| bx = BigFloat(x()) |
There was a problem hiding this comment.
how does this work? I get
julia> pi()
ERROR: MethodError: objects of type Irrational{:π} are not callable
There was a problem hiding this comment.
It seems that x == typeof(pi):
julia> @generated function foo(x) x end
foo (generic function with 1 method)
julia> foo(pi)
Irrational{:π}
julia> foo(pi)()
π = 3.1415926535897...
Is that a bug?
There was a problem hiding this comment.
right, no, this is a generated function, this is how they're supposed to work which I just keep forgetting
|
Going to crosspost this from #16513. Is this operation meaningful? Non-exact operations on Also, converting from an |
|
@TotalVerb This pr does not depart from anything. This only fixes a roundoff error (on a dubious conversion). It would be a different issue to just remove the conversion (with a @simonbyrne Given the confusions about (Maybe something like) # This is a @generated function because it allows the actual calculation to be performed compile-time
# Note that it returns a Rational constant that get's spliced into the AST of the caller |
|
At the moment, the
|
simonbyrne
force-pushed
the
sb/rational-irrational
branch
2 times, most recently
from
da37da9 to
e2100a0
Compare
Fixes #16513. Changes generated macro to pure macro, so as to avoid breaking in future compiler changes.
simonbyrne
force-pushed
the
sb/rational-irrational
branch
from
e2100a0 to
b1164b0
Compare
|
Using Should be ready to merge. |
Fixes JuliaLang#16513. Changes generated macro to pure macro, so as to avoid breaking in future compiler changes.
Fixes #16513.