Skip to content

Commit

Permalink
fix #30114, specificity transitivity errors in convert methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@JeffBezanson
JeffBezanson committed Nov 30, 2018
1 parent e4f408e commit ce59b5e
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 48 deletions.
9 changes: 3 additions & 6 deletions base/missing.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,12 @@ promote_rule(::Type{Union{Nothing, Missing, S}}, ::Type{T}) where {T,S} =
Union{Nothing, Missing, promote_type(T, S)}

convert(::Type{Union{T, Missing}}, x::Union{T, Missing}) where {T} = x
convert(::Type{Union{T, Missing}}, x) where {T} = convert(T, x)
# To print more appropriate message than "T not defined"
convert(::Type{Union{T, Missing}}, x) where {T} =
@isdefined(T) ? convert(T, x) : throw(MethodError(convert, (Missing, x)))
# To fix ambiguities
convert(::Type{Missing}, ::Missing) = missing
convert(::Type{Union{Nothing, Missing}}, x::Union{Nothing, Missing}) = x
convert(::Type{Union{Nothing, Missing, T}}, x::Union{Nothing, Missing, T}) where {T} = x
convert(::Type{Union{Nothing, Missing}}, x) =
throw(MethodError(convert, (Union{Nothing, Missing}, x)))
# To print more appropriate message than "T not defined"
convert(::Type{Missing}, x) = throw(MethodError(convert, (Missing, x)))

# Comparison operators
==(::Missing, ::Missing) = missing
Expand Down
5 changes: 2 additions & 3 deletions base/some.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ convert(::Type{Some{T}}, x::Some) where {T} = Some{T}(convert(T, x.value))
convert(::Type{Union{Some{T}, Nothing}}, x::Some) where {T} = convert(Some{T}, x)

convert(::Type{Union{T, Nothing}}, x::Union{T, Nothing}) where {T} = x
convert(::Type{Union{T, Nothing}}, x::Any) where {T} = convert(T, x)
convert(::Type{Nothing}, x::Nothing) = nothing
convert(::Type{Nothing}, x::Any) = throw(MethodError(convert, (Nothing, x)))
convert(::Type{Union{T, Nothing}}, x::Any) where {T} =
@isdefined(T) ? convert(T, x) : throw(MethodError(convert, (Nothing, x)))

function show(io::IO, x::Some)
if get(io, :typeinfo, Any) == typeof(x)
Expand Down
94 changes: 59 additions & 35 deletions src/subtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ static int obviously_disjoint(jl_value_t *a, jl_value_t *b, int specificity)
for(i=0; i < np; i++) {
jl_value_t *ai = jl_tparam(ad,i);
jl_value_t *bi = jl_tparam(bd,i);
if (!istuple && specificity) {
if (!istuple && specificity && jl_has_free_typevars(ai)) {
// X{<:SomeDataType} and X{Union{Y,Z,...}} need to be disjoint to
// avoid this transitivity problem:
// A = Tuple{Type{LinearIndices{N,R}}, LinearIndices{N}} where {N,R}
Expand Down Expand Up @@ -2585,9 +2585,8 @@ static int tuple_morespecific(jl_datatype_t *cdt, jl_datatype_t *pdt, int invari
return 1;

int cms = type_morespecific_(ce, pe, invariant, env);
int eqv = !cms && eq_msp(ce, pe, env);

if (!cms && !eqv && !sub_msp(ce, pe, env)) {
if (!cms && !sub_msp(ce, pe, env)) {
/*
A bound vararg tuple can be more specific despite disjoint elements in order to
preserve transitivity. For example in
Expand All @@ -2599,6 +2598,8 @@ static int tuple_morespecific(jl_datatype_t *cdt, jl_datatype_t *pdt, int invari
return some_morespecific && cva && ckind == JL_VARARG_BOUND && num_occurs((jl_tvar_t*)jl_tparam1(jl_unwrap_unionall(clast)), env) > 1;
}

int eqv = !cms && eq_msp(ce, pe, env);

// Tuple{..., T} not more specific than Tuple{..., Vararg{S}} if S is diagonal
if (eqv && i == clen-1 && clen == plen && !cva && pva && jl_is_typevar(ce) && jl_is_typevar(pe) && !cdiag && pdiag)
return 0;
Expand Down Expand Up @@ -2692,24 +2693,23 @@ static int num_occurs(jl_tvar_t *v, jl_typeenv_t *env)
return 0;
}

#define HANDLE_UNIONALL_A \
jl_unionall_t *ua = (jl_unionall_t*)a; \
jl_typeenv_t newenv = { ua->var, 0x0, env }; \
newenv.val = (jl_value_t*)(intptr_t)count_occurs(ua->body, ua->var); \
return type_morespecific_(ua->body, b, invariant, &newenv)

#define HANDLE_UNIONALL_B \
jl_unionall_t *ub = (jl_unionall_t*)b; \
jl_typeenv_t newenv = { ub->var, 0x0, env }; \
newenv.val = (jl_value_t*)(intptr_t)count_occurs(ub->body, ub->var); \
return type_morespecific_(a, ub->body, invariant, &newenv)

static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_typeenv_t *env)
{
if (a == b)
return 0;

if (jl_is_unionall(a)) {
jl_unionall_t *ua = (jl_unionall_t*)a;
jl_typeenv_t newenv = { ua->var, 0x0, env };
newenv.val = (jl_value_t*)(intptr_t)count_occurs(ua->body, ua->var);
return type_morespecific_(ua->body, b, invariant, &newenv);
}
if (jl_is_unionall(b)) {
jl_unionall_t *ub = (jl_unionall_t*)b;
jl_typeenv_t newenv = { ub->var, 0x0, env };
newenv.val = (jl_value_t*)(intptr_t)count_occurs(ub->body, ub->var);
return type_morespecific_(a, ub->body, invariant, &newenv);
}

if (jl_is_tuple_type(a) && jl_is_tuple_type(b)) {
// When one is JL_VARARG_BOUND and the other has fixed length,
// allow the argument length to fix the tvar
Expand All @@ -2728,6 +2728,9 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
}

if (jl_is_uniontype(a)) {
if (jl_is_unionall(b)) {
HANDLE_UNIONALL_B;
}
// Union a is more specific than b if some element of a is more specific than b, but
// not vice-versa.
if (sub_msp(b, a, env))
Expand Down Expand Up @@ -2764,6 +2767,9 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
}

if (jl_is_uniontype(b)) {
if (jl_is_unionall(a)) {
HANDLE_UNIONALL_A;
}
jl_uniontype_t *u = (jl_uniontype_t*)b;
if (type_morespecific_(a, u->a, invariant, env) || type_morespecific_(a, u->b, invariant, env))
return !type_morespecific_(b, a, invariant, env);
Expand All @@ -2772,7 +2778,7 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty

if (!invariant) {
if ((jl_datatype_t*)a == jl_any_type) return 0;
if ((jl_datatype_t*)b == jl_any_type) return 1;
if ((jl_datatype_t*)b == jl_any_type && !jl_is_typevar(a)) return 1;
}

if (jl_is_datatype(a) && jl_is_datatype(b)) {
Expand All @@ -2796,13 +2802,20 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
for(size_t i=0; i < jl_nparams(tta); i++) {
jl_value_t *apara = jl_tparam(tta,i);
jl_value_t *bpara = jl_tparam(ttb,i);
if (!jl_has_free_typevars(apara) && !jl_has_free_typevars(bpara) &&
!jl_types_equal(apara, bpara))
int afree = jl_has_free_typevars(apara);
int bfree = jl_has_free_typevars(bpara);
if (!afree && !bfree && !jl_types_equal(apara, bpara))
return 0;
if (type_morespecific_(apara, bpara, 1, env))
if (type_morespecific_(apara, bpara, 1, env) && (jl_is_typevar(apara) || !afree || bfree))
ascore += 1;
else if (type_morespecific_(bpara, apara, 1, env))
else if (type_morespecific_(bpara, apara, 1, env) && (jl_is_typevar(bpara) || !bfree || afree))
bscore += 1;
else if (eq_msp(apara, bpara, env)) {
if (!afree && bfree)
ascore += 1;
else if (afree && !bfree)
bscore += 1;
}
if (jl_is_typevar(bpara) && !jl_is_typevar(apara) && !jl_is_type(apara))
ascore1 = 1;
else if (jl_is_typevar(apara) && !jl_is_typevar(bpara) && !jl_is_type(bpara))
Expand All @@ -2828,9 +2841,6 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
return 0;
return ascore > bscore || adiag > bdiag;
}
else if (invariant) {
return 0;
}
tta = tta->super; super = 1;
}
return 0;
Expand All @@ -2852,33 +2862,47 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
if (invariant) {
if (((jl_tvar_t*)a)->ub == jl_bottom_type)
return 1;
if (jl_has_free_typevars(b)) {
if (type_morespecific_(((jl_tvar_t*)a)->ub, b, 0, env) ||
eq_msp(((jl_tvar_t*)a)->ub, b, env))
return num_occurs((jl_tvar_t*)a, env) >= 2;
}
else {
if (eq_msp(((jl_tvar_t*)a)->ub, b, env))
return num_occurs((jl_tvar_t*)a, env) >= 2;
if (!jl_has_free_typevars(b))
return 0;
}
}
return type_morespecific_((jl_value_t*)((jl_tvar_t*)a)->ub, b, 0, env);
else {
// need `{T,T} where T` more specific than `{Any, Any}`
if (b == jl_any_type && ((jl_tvar_t*)a)->ub == jl_any_type && num_occurs((jl_tvar_t*)a, env) >= 2)
return 1;
}
return type_morespecific_(((jl_tvar_t*)a)->ub, b, 0, env);
}
if (jl_is_typevar(b)) {
if (!jl_is_type(a))
return 1;
if (invariant) {
if (((jl_tvar_t*)b)->ub == jl_bottom_type)
return 0;
if (eq_msp(a, ((jl_tvar_t*)b)->ub, env))
return num_occurs((jl_tvar_t*)b, env) < 2;
if (jl_has_free_typevars(a)) {
if (type_morespecific_(a, ((jl_tvar_t*)b)->ub, 0, env) ||
eq_msp(a, ((jl_tvar_t*)b)->ub, env))
if (type_morespecific_(a, ((jl_tvar_t*)b)->ub, 0, env))
return num_occurs((jl_tvar_t*)b, env) < 2;
return 0;
}
else {
if (obviously_disjoint(a, ((jl_tvar_t*)b)->ub, 1))
return 0;
if (type_morespecific_(((jl_tvar_t*)b)->ub, a, 0, env))
return 0;
return 1;
}
}
return type_morespecific_(a, (jl_value_t*)((jl_tvar_t*)b)->ub, 0, env);
return type_morespecific_(a, ((jl_tvar_t*)b)->ub, 0, env);
}

if (jl_is_unionall(a)) {
HANDLE_UNIONALL_A;
}
if (jl_is_unionall(b)) {
HANDLE_UNIONALL_B;
}

return 0;
Expand Down
63 changes: 59 additions & 4 deletions test/specificity.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ let
@test !args_morespecific(b2, a)
a = Tuple{Type{T1}, Ptr{T1}} where T1<:Integer
b2 = Tuple{Type{T2}, Ptr{Integer}} where T2<:Integer
@test !args_morespecific(a, b2)
@test args_morespecific(b2, a)
@test args_morespecific(a, b2)
@test !args_morespecific(b2, a)
end

# issue #11534
Expand Down Expand Up @@ -108,7 +108,7 @@ f17016(f, t::T_17016) = 0
f17016(f, t1::Tuple) = 1
@test f17016(0, (1,2,3)) == 0

@test args_morespecific(Tuple{Type{Any}, Any}, Tuple{Type{T}, Any} where T<:VecElement)
@test !args_morespecific(Tuple{Type{Any}, Any}, Tuple{Type{T}, Any} where T<:VecElement)
@test !args_morespecific((Tuple{Type{T}, Any} where T<:VecElement), Tuple{Type{Any}, Any})

@test !args_morespecific(Tuple{Type{T}, Tuple{Any, Vararg{Any, N} where N}} where T<:Tuple{Any, Vararg{Any, N} where N},
Expand Down Expand Up @@ -212,7 +212,7 @@ f27361(::M) where M <: Tuple{3} = nothing
@test args_morespecific(Tuple{Core.TypeofBottom}, Tuple{DataType})
@test args_morespecific(Tuple{Core.TypeofBottom}, Tuple{Type{<:Tuple}})

@test args_morespecific(Tuple{Type{Any}, Type}, Tuple{Type{T}, Type{T}} where T)
@test args_morespecific(Tuple{Type{T}, Type{T}} where T, Tuple{Type{Any}, Type})
@test !args_morespecific(Tuple{Type{Any}, Type}, Tuple{Type{T}, Type{T}} where T<:Union{})

# issue #22592
Expand Down Expand Up @@ -240,3 +240,58 @@ end
@test args_morespecific(Tuple{Array,Int64}, Tuple{Array,Vararg{Int64,N}} where N)
@test args_morespecific(Tuple{Array,Int64}, Tuple{Array,Vararg{Int64,N} where N})
@test !args_morespecific(Tuple{Array,Int64}, Tuple{AbstractArray, Array})

# issue #30114
let T1 = Tuple{Type{Tuple{Vararg{AbstractUnitRange{Int64},N} where N}},CartesianIndices{N,R} where R<:Tuple{Vararg{AbstractUnitRange{Int64},N}}} where N
T2 = Tuple{Type{T},T} where T<:AbstractArray
T3 = Tuple{Type{AbstractArray{T,N} where N},AbstractArray} where T
T4 = Tuple{Type{AbstractArray{T,N}},AbstractArray{s57,N} where s57} where N where T
@test !args_morespecific(T1, T2)
@test !args_morespecific(T1, T3)
@test !args_morespecific(T1, T4)
@test args_morespecific(T2, T3)
@test args_morespecific(T2, T4)
end

@test !args_morespecific(Tuple{Type{Tuple{Vararg{AbstractUnitRange{Int64},N}}},} where N,
Tuple{Type{Tuple{Vararg{AbstractUnitRange,N} where N}},})

@test args_morespecific(Tuple{Type{SubArray{T,2,P} where T}, Array{T}} where T where P,
Tuple{Type{AbstractArray{T,N} where N},AbstractArray} where T)

@test args_morespecific(Tuple{Type{T},T} where T<:BitArray,
Tuple{Type{BitArray},Any})

abstract type Domain{T} end

abstract type AbstractInterval{T} <: Domain{T} end

struct Interval{L,R,T} <: AbstractInterval{T}
end

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

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

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

@test args_morespecific(Tuple{Type{Missing},Any},
Tuple{Type{Union{Nothing, T}},Any} where T)

0 comments on commit ce59b5e

Please sign in to comment.