Skip to content

Commit

Permalink
Fix ambiguity warning (Rational->BigFloat conversion)
Browse files Browse the repository at this point in the history
  • Loading branch information
@carlobaldassi
carlobaldassi committed Apr 2, 2014
1 parent d265f4d commit a85476b
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion base/rational.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ convert(::Type{Bool}, x::Rational) = (x!=0) # to resolve ambiguity
convert{T<:Integer}(::Type{T}, x::Rational) = (isinteger(x) ? convert(T, x.num) : throw(InexactError()))

convert(::Type{FloatingPoint}, x::Rational) = float(x.num)/float(x.den)
function convert{T<:FloatingPoint,S}(::Type{T}, x::Rational{S})
function convert{T<:FloatingPoint,S<:Integer}(::Type{T}, x::Rational{S})
P = promote_type(T,S)
convert(P,x.num)/convert(P,x.den)
end
Expand Down

3 comments on commit a85476b

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. That was an oversight.

@carlobaldassi
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix was wrong BTW (sorry); the next commit is the one which actually works. (alternative fix: as in this commit, but also annotate the previous method as convert{S<:Integer}(::Type{FloatingPoint}, x::Rational{S}))
The ambiguity comes from the fact that in method definitions Rational is equivalent to Rational{S<:Integer} (as per the type definition), while Rational{S} is seen as more general (although it can't possibly be).

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be an issue about this: dispatch should automatically restrict Rational{S} to Rational{S<:Integer}.

Please sign in to comment.