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

1 participant
@JeffBezanson
Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2018

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

@JeffBezanson

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2018

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 Nov 29, 2018

@JeffBezanson JeffBezanson force-pushed the jb/fix30114 branch from 2a72d52 to 39d40ed Nov 30, 2018

@JeffBezanson

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2018

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 merged commit 6594acc into master Dec 5, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
julia freebsd ci Build done
Details

@JeffBezanson JeffBezanson deleted the jb/fix30114 branch Dec 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.