Skip to content

Commit

Permalink
fix some inference and method lookup issues
Browse files Browse the repository at this point in the history
- better inference of type applications with unknown arguments
- fix inference of Tuple{::Any}, which might become a vararg tuple
- allow calling UnionAlls in abstract_call_gf_by_type
- fix static method lookup with the definitions
  `f{T}(x::T...) = 0`
  `f(x...) = ""`
- remove redundant call to jl_subtype in ml_matches
- clean up some code subsumed by the new subtyping algorithm
- when matching `Type{T}` in the cache, require `typeof(T)` to match
- improve intersection of `BottomType` and `Type{T} where T<:S`
  • Loading branch information
JeffBezanson committed Jan 26, 2017
1 parent 32d0645 commit 93a725d
Show file tree
Hide file tree
Showing 11 changed files with 196 additions and 113 deletions.
16 changes: 16 additions & 0 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,22 @@ function rewrap_unionall(t::ANY, u::ANY)
return UnionAll(u.var, rewrap_unionall(t, u.body))
end

# replace TypeVars in all enclosing UnionAlls with fresh TypeVars
function rename_unionall(u::ANY)
if !isa(u,UnionAll)
return u
end
body = rename_unionall(u.body)
if body === u.body
body = u
else
body = UnionAll(u.var, body)
end
var = u.var::TypeVar
nv = TypeVar(var.name, var.lb, var.ub)
return UnionAll(nv, body{nv})
end

const _va_typename = Vararg.body.body.name
function isvarargtype(t::ANY)
t = unwrap_unionall(t)
Expand Down
75 changes: 55 additions & 20 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ function instanceof_tfunc(t::ANY)
end
elseif isType(t)
return t.parameters[1]
elseif isa(t,UnionAll)
t′ = unwrap_unionall(t)
if isType(t′)
return rewrap_unionall(t′.parameters[1], t)
end
end
return Any
end
Expand Down Expand Up @@ -613,9 +618,9 @@ add_tfunc(isa, 2, 2,
end)
add_tfunc(issubtype, 2, 2,
function (a::ANY, b::ANY)
a = instanceof_tfunc(a)
b = instanceof_tfunc(b)
if a !== Any && b !== Any
if (isa(a,Const) || isType(a)) && (isa(b,Const) || isType(b))
a = instanceof_tfunc(a)
b = instanceof_tfunc(b)
if !has_free_typevars(a) && !has_free_typevars(b)
return Const(issubtype(a, b))
end
Expand Down Expand Up @@ -862,27 +867,45 @@ function apply_type_tfunc(headtypetype::ANY, args::ANY...)
end
uncertain = false
tparams = Any[]
outervars = Any[]
for i = 1:largs
ai = args[i]
if isType(ai)
aip1 = ai.parameters[1]
uncertain |= has_free_typevars(aip1)
push!(tparams, aip1)
elseif isa(ai, Const) && (isa(ai.val, Type) || valid_tparam(ai.val))
push!(tparams, ai.val)
else
# TODO: return `Bottom` for trying to apply a non-UnionAll
#if !istuple && i-1 > length(headtype.parameters)
# # too many parameters for type
# return Bottom
#end
uncertain = true
# These blocks improve type info but make compilation a bit slower.
# XXX
#unw = unwrap_unionall(ai)
#isT = isType(unw)
#if isT && isa(ai,UnionAll) && contains_is(outervars, ai.var)
# ai = rename_unionall(ai)
# unw = unwrap_unionall(ai)
#end
if istuple
push!(tparams, Any)
if i == largs
push!(tparams, Vararg)
# XXX
#elseif isT
# push!(tparams, rewrap_unionall(unw.parameters[1], ai))
else
push!(tparams, Any)
end
# XXX
#elseif isT
# push!(tparams, unw.parameters[1])
# while isa(ai, UnionAll)
# push!(outervars, ai.var)
# ai = ai.body
# end
else
# TODO: use rewrap_unionall to skip only the unknown parameters
#push!(tparams, headtype.parameters[i-1])
break
v = TypeVar(:_)
push!(tparams, v)
push!(outervars, v)
end
end
end
Expand All @@ -892,14 +915,20 @@ function apply_type_tfunc(headtypetype::ANY, args::ANY...)
catch ex
# type instantiation might fail if one of the type parameters
# doesn't match, which could happen if a type estimate is too coarse
appl = headtype
uncertain = true
return Type{_} where _<:headtype
end
!uncertain && return Const(appl)
if type_too_complex(appl,0)
return Type{_} where _<:headtype
end
!isa(appl,TypeVar) ? (Type{_} where _<:appl) : Type{appl}
if istuple
return Type{_} where _<:appl
end
ans = Type{appl}
for i = length(outervars):-1:1
ans = UnionAll(outervars[i], ans)
end
return ans
end
add_tfunc(apply_type, 1, IInf, apply_type_tfunc)

Expand Down Expand Up @@ -1098,10 +1127,11 @@ function abstract_call_gf_by_type(f::ANY, atype::ANY, sv::InferenceState)
# here I picked 4.
argtype = limit_tuple_type(atype, sv.params)
argtypes = unwrap_unionall(argtype).parameters
ft = argtypes[1] # TODO: ccall jl_first_argument_datatype here
ft = unwrap_unionall(argtypes[1]) # TODO: ccall jl_first_argument_datatype here
isa(ft, DataType) || return Any # the function being called is unknown. can't properly handle this backedge right now
isdefined(ft.name, :mt) || return Any # not callable. should be Bottom, but can't track this backedge right now
if ft.name === _Type_name
ftname = ft.name
isdefined(ftname, :mt) || return Any # not callable. should be Bottom, but can't track this backedge right now
if ftname === _Type_name
tname = ft.parameters[1]
if isa(tname, TypeVar)
tname = tname.ub
Expand Down Expand Up @@ -1226,7 +1256,7 @@ function abstract_call_gf_by_type(f::ANY, atype::ANY, sv::InferenceState)

# if sig changed, may need to recompute the sparams environment
if recomputesvec && !isempty(sparams)
recomputed = ccall(:jl_type_intersection_env, Ref{SimpleVector}, (Any, Any), sig, method.sig)
recomputed = ccall(:jl_env_from_type_intersection, Ref{SimpleVector}, (Any, Any), sig, method.sig)
sig = recomputed[1]
if !isa(unwrap_unionall(sig), DataType) # probably Union{}
rettype = Any
Expand All @@ -1243,7 +1273,7 @@ function abstract_call_gf_by_type(f::ANY, atype::ANY, sv::InferenceState)
if !(fullmatch || rettype === Any)
# also need an edge to the method table in case something gets
# added that did not intersect with any existing method
add_mt_backedge(ft.name.mt, argtype, sv)
add_mt_backedge(ftname.mt, argtype, sv)
update_valid_age!(min_valid[1], max_valid[1], sv)
end
if isempty(x)
Expand Down Expand Up @@ -1655,6 +1685,11 @@ function abstract_eval(e::ANY, vtypes::VarTable, sv::InferenceState)
t = Bottom # a return type of Box{Any} is invalid
end
end
for v in sv.linfo.sparam_vals
if isa(v,TypeVar)
t = UnionAll(v, t)
end
end
else
t = Any
end
Expand Down
2 changes: 1 addition & 1 deletion src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -2885,7 +2885,7 @@ jl_method_instance_t *jl_recache_method_instance(jl_method_instance_t *li, size_
jl_datatype_t *argtypes = (jl_datatype_t*)li->specTypes;
jl_set_typeof(li, (void*)(intptr_t)0x40); // invalidate the old value to help catch errors
jl_svec_t *env = jl_emptysvec;
jl_value_t *ti = jl_type_intersection_matching((jl_value_t*)argtypes, (jl_value_t*)m->sig, &env);
jl_value_t *ti = jl_type_intersection_env((jl_value_t*)argtypes, (jl_value_t*)m->sig, &env);
//assert(ti != jl_bottom_type); (void)ti;
if (ti == jl_bottom_type)
env = jl_emptysvec; // the intersection may fail now if the type system had made an incorrect subtype env in the past
Expand Down
98 changes: 52 additions & 46 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,7 @@ jl_tupletype_t *arg_type_tuple(jl_value_t **args, size_t nargs)
}

jl_method_instance_t *jl_method_lookup_by_type(jl_methtable_t *mt, jl_tupletype_t *types,
int cache, int inexact, int allow_exec, size_t world)
int cache, int inexact, int allow_exec, size_t world)
{
jl_typemap_entry_t *entry = jl_typemap_assoc_by_type(mt->cache, types, NULL, 0, 1, jl_cachearg_offset(mt), world);
if (entry) {
Expand Down Expand Up @@ -2276,7 +2276,7 @@ jl_value_t *jl_gf_invoke(jl_tupletype_t *types0, jl_value_t **args, size_t nargs
else {
tt = arg_type_tuple(args, nargs);
if (entry->tvars != jl_emptysvec) {
jl_value_t *ti = jl_lookup_match((jl_value_t*)tt, (jl_value_t*)entry->sig, &tpenv);
jl_value_t *ti = jl_type_intersection_env((jl_value_t*)tt, (jl_value_t*)entry->sig, &tpenv);
assert(ti != (jl_value_t*)jl_bottom_type);
(void)ti;
}
Expand Down Expand Up @@ -2359,7 +2359,7 @@ JL_DLLEXPORT jl_value_t *jl_get_invoke_lambda(jl_methtable_t *mt,
JL_GC_PUSH2(&tpenv, &sig);
if (entry->tvars != jl_emptysvec) {
jl_value_t *ti =
jl_lookup_match((jl_value_t*)tt, (jl_value_t*)entry->sig, &tpenv);
jl_type_intersection_env((jl_value_t*)tt, (jl_value_t*)entry->sig, &tpenv);
assert(ti != (jl_value_t*)jl_bottom_type);
(void)ti;
}
Expand Down Expand Up @@ -2431,7 +2431,7 @@ JL_DLLEXPORT jl_svec_t *jl_match_method(jl_value_t *type, jl_value_t *sig)
jl_svec_t *env = jl_emptysvec;
jl_value_t *ti=NULL;
JL_GC_PUSH2(&env, &ti);
ti = jl_lookup_match(type, (jl_value_t*)sig, &env);
ti = jl_type_intersection_env(type, (jl_value_t*)sig, &env);
jl_svec_t *result = jl_svec2(ti, env);
JL_GC_POP();
return result;
Expand All @@ -2440,28 +2440,48 @@ JL_DLLEXPORT jl_svec_t *jl_match_method(jl_value_t *type, jl_value_t *sig)
// Determine whether a typevar exists inside at most one DataType.
// These are the typevars that will always be matched by any matching
// arguments.
static int tvar_exists_at_top_level(jl_value_t *tv, jl_tupletype_t *sig, int attop)
static int tvar_exists_at_top_level(jl_tvar_t *tv, jl_value_t *sig)
{
sig = (jl_tupletype_t*)jl_unwrap_unionall((jl_value_t*)sig);
sig = jl_unwrap_unionall(sig);
int i, l=jl_nparams(sig);
for(i=0; i < l; i++) {
jl_value_t *a = jl_tparam(sig, i);
a = jl_unwrap_unionall(a);
if (jl_is_vararg_type(a))
a = jl_unwrap_vararg(a);
if (a == tv)
return 0;
if (jl_is_type_type(a))
a = jl_unwrap_unionall(jl_tparam0(a));
if (a == (jl_value_t*)tv)
return 1;
if (attop && jl_is_datatype(a)) {
if (jl_is_datatype(a)) {
jl_svec_t *p = ((jl_datatype_t*)a)->parameters;
int j;
for(j=0; j < jl_svec_len(p); j++) {
if (jl_svecref(p,j) == tv)
if (jl_svecref(p,j) == (jl_value_t*)tv)
return 1;
}
}
}
return 0;
}

static int matched_all_tvars(jl_value_t *sig, jl_value_t **env, int envsz)
{
size_t i;
jl_value_t *temp = sig;
for (i = 0; i < envsz; i++) {
assert(jl_is_unionall(temp));
if (jl_is_typevar(env[i]) &&
// if tvar is at the top level it will definitely be matched.
// see issue #5575
!tvar_exists_at_top_level(((jl_unionall_t*)temp)->var, sig)) {
return 0;
}
temp = ((jl_unionall_t*)temp)->body;
}
return 1;
}

struct ml_matches_env {
struct typemap_intersection_env match;
// results:
Expand Down Expand Up @@ -2516,7 +2536,8 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio
// the "limited" mode used by type inference.
for (i = 0; i < len; i++) {
jl_value_t *prior_ti = jl_svecref(jl_array_ptr_ref(closure->t, i), 0);
// TODO: should be possible to remove the `jl_is_leaf_type` check
// TODO: should be possible to remove the `jl_is_leaf_type` check,
// but we still need it in case an intersection was approximate.
if (jl_is_leaf_type(prior_ti) && jl_subtype(closure->match.ti, prior_ti)) {
skip = 1;
break;
Expand All @@ -2536,46 +2557,31 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio
if (!skip) {
/*
Check whether all static parameters matched. If not, then we
have an argument type like Vector{T{Int,_}}, and a signature like
f{A,B}(::Vector{T{A,B}}). If "_" turns out to be a non-typevar
at runtime then this method matches, otherwise it doesn't. So we
have to look for more matches. This caused issue #4731.
might have argument type `Tuple{}` and signature type `Tuple{Vararg{T}}`,
which doesn't match due to there being no value for `T`.
This is a remaining case of issue #4731 after introducing UnionAll.
*/
int matched_all_typevars = 1;
size_t l = jl_svec_len(closure->match.env);
for (i = 0; i < l; i++) {
jl_value_t *tv;
if (jl_is_typevar(ml->tvars))
tv = (jl_value_t*)ml->tvars;
int done = 0, return_this_match = 1;
jl_svec_t *env = closure->match.env;
if (closure0->issubty) {
// if the queried type is a subtype, but not all tvars matched, then
// this method is excluded by the static-parameters-must-have-values rule
if (!matched_all_tvars((jl_value_t*)ml->sig, jl_svec_data(env), jl_svec_len(env)))
return_this_match = !jl_is_leaf_type(closure->match.type);
else
tv = jl_svecref(ml->tvars, i);
if (jl_is_typevar(jl_svecref(closure->match.env, i)) &&
// if tvar is at the top level it will definitely be matched.
// see issue #5575
!tvar_exists_at_top_level(tv, ml->sig, 1)) {
matched_all_typevars = 0;
break;
}
done = 1; // stop; signature fully covers queried type
}
int done = 0, return_this_match = 1;
// (type ∩ ml->sig == type) ⇒ (type ⊆ ml->sig)
// NOTE: jl_subtype check added in case the intersection is
// over-approximated.
if (matched_all_typevars && jl_types_equal(closure->match.ti, closure->match.type) &&
jl_subtype(closure->match.type, (jl_value_t*)ml->sig)) {
done = 1; // terminate visiting method list
}
// here we have reached a definition that fully covers the arguments.
// however, if there are ambiguities this method might not actually
// match, so we shouldn't add it to the results.
if (meth->ambig != jl_nothing && (!closure->include_ambiguous || done)) {
// if we reach a definition that fully covers the arguments but there are
// ambiguities, then this method might not actually match, so we shouldn't
// add it to the results.
if (return_this_match && meth->ambig != jl_nothing && (!closure->include_ambiguous || done)) {
jl_svec_t *env = NULL;
JL_GC_PUSH1(&env);
for (size_t j = 0; j < jl_array_len(meth->ambig); j++) {
jl_method_t *mambig = (jl_method_t*)jl_array_ptr_ref(meth->ambig, j);
env = jl_emptysvec;
jl_value_t *mti = jl_type_intersection_matching((jl_value_t*)closure->match.type,
(jl_value_t*)mambig->sig, &env);
jl_value_t *mti = jl_type_intersection_env((jl_value_t*)closure->match.type,
(jl_value_t*)mambig->sig, &env);
if (mti != (jl_value_t*)jl_bottom_type) {
if (closure->include_ambiguous) {
assert(done);
Expand All @@ -2589,15 +2595,15 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio
closure->t = (jl_value_t*)jl_alloc_vec_any(0);
}
jl_array_ptr_1d_push((jl_array_t*)closure->t,
(jl_value_t*)jl_svec(3, mti, env, mambig));
(jl_value_t*)jl_svec(3, mti, env, mambig));
len++;
}
}
else {
// the current method doesn't match if there is an intersection with an
// ambiguous method that covers our intersection with this one.
jl_value_t *ambi = jl_type_intersection_matching((jl_value_t*)ml->sig,
(jl_value_t*)mambig->sig, &env);
jl_value_t *ambi = jl_type_intersection_env((jl_value_t*)ml->sig,
(jl_value_t*)mambig->sig, &env);
if (jl_subtype(closure->match.ti, ambi)) {
return_this_match = 0;
break;
Expand Down
8 changes: 8 additions & 0 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,14 @@ static int typekey_eq(jl_datatype_t *tt, jl_value_t **key, size_t n)
size_t j;
size_t tnp = jl_nparams(tt);
if (n != tnp) return 0;
if (tt->name == jl_type_typename) {
// for Type{T}, require `typeof(T)` to match also, to avoid incorrect
// dispatch from changing the type of something.
// this should work because `Type`s don't have uids, and aren't the
// direct tags of values so we don't rely on pointer equality.
jl_value_t *kj = key[0], *tj = jl_tparam0(tt);
return (kj == tj || (jl_typeof(tj) == jl_typeof(kj) && jl_types_equal(tj, kj)));
}
for(j=0; j < n; j++) {
jl_value_t *kj = key[j], *tj = jl_svecref(tt->parameters,j);
if (tj != kj && !jl_types_equal(tj, kj))
Expand Down
Loading

0 comments on commit 93a725d

Please sign in to comment.