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

fix #30114, specificity transitivity errors in convert methods #30160

Merged
merged 1 commit into from
Dec 5, 2018

Conversation

JeffBezanson
Copy link
Member

@JeffBezanson JeffBezanson commented Nov 27, 2018

After this, transitivity holds for all convert methods. Fortunately, this leaves all existing specificity and ambiguity tests passing, so this is potentially backportable if PkgEval agrees. Unfortunately, there are more transitivity failures in other functions (that I will soon address in another PR), and so far the fix for them seems to unavoidably change ambiguities.

Fixes #30114.

@JeffBezanson JeffBezanson added types and dispatch Types, subtyping and method dispatch bugfix This change fixes an existing bug labels Nov 27, 2018
@JeffBezanson
Copy link
Member Author

I think I can improve this a bit by picking over a piece of #30171.

@JeffBezanson
Copy link
Member Author

Ok, the following test case works on master but fails on this branch:

let A = Tuple{Type{Interval{:closed,:closed,T} where T}, Interval{:closed,:closed,T} where T},
    B = Tuple{Type{II},                                  AbstractInterval} where II<:(Interval{:closed,:closed,T} where T),
    C = Tuple{Type{AbstractInterval},                    AbstractInterval}
    @test  args_morespecific(A, B)
    @test !args_morespecific(B, C)
    @test !args_morespecific(A, C)
end

I'll push an update that fixes it, at the cost of changing the result of 2 cases in the specificity tests. No new ambiguities though, so still pretty good.

@JeffBezanson JeffBezanson force-pushed the jb/fix30114 branch 5 times, most recently from ce59b5e to 2a72d52 Compare November 30, 2018 05:19
@JeffBezanson
Copy link
Member Author

Ok, I think I found a pretty good version of this where the only major change is that

(::Type{T})(x::T) where {T<:BitArray} = ...
BitArray(itr) = ...

is now ambiguous. That seems reasonable enough to me.

@JeffBezanson JeffBezanson added the minor change Marginal behavior change acceptable for a minor release label Nov 30, 2018
@JeffBezanson JeffBezanson merged commit 6594acc into master Dec 5, 2018
@JeffBezanson JeffBezanson deleted the jb/fix30114 branch December 5, 2018 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug minor change Marginal behavior change acceptable for a minor release types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specificity transitivity errors among convert methods
1 participant