Skip to content
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

Return type of *(::Rational, ::Integer) changed in 1.5 #36277

Closed
sostock opened this issue Jun 14, 2020 · 2 comments · Fixed by #36279
Closed

Return type of *(::Rational, ::Integer) changed in 1.5 #36277

sostock opened this issue Jun 14, 2020 · 2 comments · Fixed by #36279
Labels
regression Regression in behavior compared to a previous version
Milestone

Comments

@sostock
Copy link
Contributor

sostock commented Jun 14, 2020

#35483 introduced a new method for *(::Rational, ::Integer) which changed the return type of that operation. There was no specialized method for this before, so promotion was used.

Julia 1.4.2:

julia> Rational{Int64}(1,1) * UInt8(1) # returns Rational{Int64}
1//1

Julia 1.5.0-beta1:

julia> Rational{Int64}(1,1) * UInt8(1) # returns Rational{UInt64}
0x0000000000000001//0x0000000000000001

Should this return type be preserved between minor versions?

@Liozou
Copy link
Member

Liozou commented Jun 14, 2020

I don't know if return types should be preserved in general, but for this particular case I would assume so, since, for this very reason, some computations that used to work now error:

Julia 1.0.5 (and probably up to 1.4.x)

julia> 0x1 * -1//1
-1//1

Julia 1.5.0-beta1:

julia> 0x1 * -1//1
ERROR: InexactError: check_top_bit(UInt64, -1)
Stacktrace:
 [1] throw_inexacterror(::Symbol, ::Type{UInt64}, ::Int64) at ./boot.jl:558
 [2] check_top_bit at ./boot.jl:572 [inlined]
 [3] toUInt64 at ./boot.jl:683 [inlined]
 [4] UInt64 at ./boot.jl:713 [inlined]
 [5] convert at ./number.jl:7 [inlined]
 [6] _promote at ./promotion.jl:259 [inlined]
 [7] promote at ./promotion.jl:282 [inlined]
 [8] checked_mul at ./checked.jl:21 [inlined]
 [9] *(::UInt8, ::Rational{Int64}) at /home/liozou/julia/base/rational.jl:318
 [10] top-level scope at none:1

A solution consists in using promotions when doing binary operations: see PR #36279

@StefanKarpinski StefanKarpinski added the regression Regression in behavior compared to a previous version label Jun 16, 2020
@StefanKarpinski StefanKarpinski added this to the 1.5 milestone Jun 16, 2020
@StefanKarpinski
Copy link
Member

This seems like a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants