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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Language changes
to the `Core` module ([#29968]).
* Using the same name for both a local variable and a static parameter is now an error instead
of a warning ([#29429]).
* Method signatures such as
`f(::Type{T}, ::T) where {T <: X}` and
`f(::Type{X}, ::Any)`
are now considered ambiguous. Previously a bug caused the first one to be considered more specific ([#30160]).

New library functions
---------------------
Expand Down
1 change: 1 addition & 0 deletions base/bitarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ reinterpret(B::BitArray, dims::NTuple{N,Int}) where {N} = reshape(B, dims)

if nameof(@__MODULE__) === :Base # avoid method overwrite
(::Type{T})(x::T) where {T<:BitArray} = copy(x)
BitArray(x::BitArray) = copy(x)
end

"""
Expand Down
99 changes: 61 additions & 38 deletions src/subtype.c
View file
Open in desktop
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 @@ -2600,7 +2599,8 @@ static int tuple_morespecific(jl_datatype_t *cdt, jl_datatype_t *pdt, int invari
}

// 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)
if (!cms && i == clen-1 && clen == plen && !cva && pva && eq_msp(ce, pe, env) &&
jl_is_typevar(ce) && jl_is_typevar(pe) && !cdiag && pdiag)
return 0;

if (cms) some_morespecific = 1;
Expand Down Expand Up @@ -2692,24 +2692,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 @@ -2727,7 +2726,15 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
return tuple_morespecific((jl_datatype_t*)a, (jl_datatype_t*)b, invariant, env);
}

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

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,17 +2771,15 @@ 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);
return 0;
}

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

if (jl_is_datatype(a) && jl_is_datatype(b)) {
jl_datatype_t *tta = (jl_datatype_t*)a, *ttb = (jl_datatype_t*)b;
// Type{Union{}} is more specific than other types, so TypeofBottom must be too
Expand All @@ -2796,13 +2801,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 +2840,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,16 +2861,18 @@ 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 (!jl_has_free_typevars(b))
return 0;
}
if (eq_msp(((jl_tvar_t*)a)->ub, b, env))
return num_occurs((jl_tvar_t*)a, env) >= 2;
}
else {
// need `{T,T} where T` more specific than `{Any, Any}`
if (b == (jl_value_t*)jl_any_type && ((jl_tvar_t*)a)->ub == (jl_value_t*)jl_any_type &&
num_occurs((jl_tvar_t*)a, env) >= 2)
return 1;
}
return type_morespecific_((jl_value_t*)((jl_tvar_t*)a)->ub, b, 0, env);
return type_morespecific_(((jl_tvar_t*)a)->ub, b, 0, env);
}
if (jl_is_typevar(b)) {
if (!jl_is_type(a))
Expand All @@ -2873,12 +2884,24 @@ static int type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant, jl_ty
if (type_morespecific_(a, ((jl_tvar_t*)b)->ub, 0, env) ||
eq_msp(a, ((jl_tvar_t*)b)->ub, 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
60 changes: 59 additions & 1 deletion test/specificity.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
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 @@ -240,3 +240,61 @@ 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)

# these are ambiguous
@test !args_morespecific(Tuple{Type{T},T} where T<:BitArray,
Tuple{Type{BitArray},Any})
@test !args_morespecific(Tuple{Type{BitArray},Any},
Tuple{Type{T},T} where T<:BitArray)

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)