Skip to content

Commit

Permalink
remove additional rule that parameters must match for dispath
Browse files Browse the repository at this point in the history
matching a method signature is a simple subtyping test,
with no additional rule to ensure that all parameters have a value

and adds a Test for possibly unbound parameters,
which allows detecting methods definitions that this change affects
  • Loading branch information
vtjnash committed Aug 3, 2017
1 parent a6c44bf commit d0381be
Show file tree
Hide file tree
Showing 19 changed files with 520 additions and 318 deletions.
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ Language changes
* `{ }` expressions now use `braces` and `bracescat` as expression heads instead
of `cell1d` and `cell2d`, and parse similarly to `vect` and `vcat` ([#8470]).

* Dispatch rules have been simplified:
matching methods is now determined exclusively by subtyping;
the rule that method type parameters must be also be captured has been removed.
Instead, attempting to access the uncontrained parameters will throw an `UndefVarError`.
Linting in package tests is recommended to confirm that the set of methods
which might throw `UndefVarError` when accessing the static parameters
(`need_to_handle_undef_sparam = Set{Any}(m.sig for m in Test.detect_unbound_args(Base, recursive=true))`)
is equal (`==`) to some known set (`expected = Set()`). ([#TBD])


Breaking changes
----------------

Expand Down
60 changes: 49 additions & 11 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,6 @@ end
convert(::Type{Any}, @nospecialize(x)) = x
convert(::Type{T}, x::T) where {T} = x

convert(::Type{Tuple{}}, ::Tuple{}) = ()
convert(::Type{Tuple}, x::Tuple) = x
convert(::Type{Tuple{Vararg{T}}}, x::Tuple) where {T} = cnvt_all(T, x...)
cnvt_all(T) = ()
cnvt_all(T, x, rest...) = tuple(convert(T,x), cnvt_all(T, rest...)...)

"""
@eval [mod,] ex
Expand All @@ -86,13 +80,24 @@ end
argtail(x, rest...) = rest
tail(x::Tuple) = argtail(x...)

# TODO: a better / more infer-able definition would pehaps be
# tuple_type_head(T::Type) = fieldtype(T::Type{<:Tuple}, 1)
tuple_type_head(T::UnionAll) = (@_pure_meta; UnionAll(T.var, tuple_type_head(T.body)))
function tuple_type_head(T::Union)
@_pure_meta
return Union{tuple_type_head(T.a), tuple_type_head(T.b)}
end
function tuple_type_head(T::DataType)
@_pure_meta
T.name === Tuple.name || throw(MethodError(tuple_type_head, (T,)))
return unwrapva(T.parameters[1])
end

tuple_type_tail(T::UnionAll) = (@_pure_meta; UnionAll(T.var, tuple_type_tail(T.body)))
function tuple_type_tail(T::Union)
@_pure_meta
return Union{tuple_type_tail(T.a), tuple_type_tail(T.b)}
end
function tuple_type_tail(T::DataType)
@_pure_meta
T.name === Tuple.name || throw(MethodError(tuple_type_tail, (T,)))
Expand Down Expand Up @@ -159,11 +164,44 @@ function typename(a::Union)
end
typename(union::UnionAll) = typename(union.body)

convert(::Type{T}, x::Tuple{Any,Vararg{Any}}) where {T<:Tuple{Any,Vararg{Any}}} =
tuple(convert(tuple_type_head(T),x[1]), convert(tuple_type_tail(T), tail(x))...)
convert(::Type{T}, x::T) where {T<:Tuple{Any,Vararg{Any}}} = x

oftype(x,c) = convert(typeof(x),c)
convert(::Type{T}, x::T) where {T<:Tuple{Any, Vararg{Any}}} = x
convert(::Type{T}, x::Tuple{Any, Vararg{Any}}) where {T<:Tuple} =
(convert(tuple_type_head(T), x[1]), convert(tuple_type_tail(T), tail(x))...)

# TODO: the following definitions are equivalent (behaviorally) to the above method
# I think they may be faster / more efficient for inference,
# if we could enable them, but are they?
# TODO: These currently can't be used (#21026) since with
# z(::Type{Tuple{Val{T}} where T}) = T
# calling
# z(Tuple{Val})
# fails, even though `Type{Tuple{Val}} == Type{Tuple{Val{S}} where S}`
# and so T should be Val{S} where S
#convert(_::Type{Tuple{S}}, x::Tuple{S}) where {S} = x
#convert(_::Type{Tuple{S}}, x::Tuple{Any}) where {S} = (convert(S, x[1]),)
#convert(_::Type{T}, x::T) where {S, N, T<:Tuple{S, Vararg{S, N}}} = x
#convert(_::Type{Tuple{S, Vararg{S, N}}},
# x::Tuple{Any, Vararg{Any, N}}) where
# {S, N} = cnvt_all(S, x...)
#convert(_::Type{Tuple{Vararg{S, N}}},
# x::Tuple{Vararg{Any, N}}) where
# {S, N} = cnvt_all(S, x...)
# TODO: These currently can't be used since
# Type{NTuple} <: (Type{Tuple{Vararg{S}}} where S) is true
# even though the value S doesn't exist
#convert(_::Type{Tuple{Vararg{S}}},
# x::Tuple{Any, Vararg{Any}}) where
# {S} = cnvt_all(S, x...)
#convert(_::Type{Tuple{Vararg{S}}},
# x::Tuple{Vararg{Any}}) where
# {S} = cnvt_all(S, x...)
#cnvt_all(T) = ()
#cnvt_all(T, x, rest...) = (convert(T, x), cnvt_all(T, rest...)...)
# TODO: These may be necessary if the above are enabled
#convert(::Type{Tuple{}}, ::Tuple{}) = ()
#convert(::Type{Tuple{Vararg{S}}} where S, ::Tuple{}) = ()

oftype(x, c) = convert(typeof(x), c)

unsigned(x::Int) = reinterpret(UInt, x)
signed(x::UInt) = reinterpret(Int, x)
Expand Down
54 changes: 33 additions & 21 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1448,8 +1448,7 @@ function invoke_tfunc(@nospecialize(f), @nospecialize(types), @nospecialize(argt
return Any
end
meth = entry.func
(ti, env) = ccall(:jl_match_method, Ref{SimpleVector}, (Any, Any),
argtype, meth.sig)
(ti, env) = ccall(:jl_env_from_type_intersection, Any, (Any, Any), argtype, meth.sig)::SimpleVector
rt, edge = typeinf_edge(meth::Method, ti, env, sv)
edge !== nothing && add_backedge!(edge::MethodInstance, sv)
return rt
Expand Down Expand Up @@ -1826,7 +1825,7 @@ function abstract_call_method(method::Method, @nospecialize(f), @nospecialize(si

# if sig changed, may need to recompute the sparams environment
if isa(method.sig, UnionAll) && isempty(sparams)
recomputed = ccall(:jl_env_from_type_intersection, Ref{SimpleVector}, (Any, Any), sig, method.sig)
recomputed = ccall(:jl_match_method, Any, (Any, Any), sig, method.sig)::SimpleVector
sig = recomputed[1]
if !isa(unwrap_unionall(sig), DataType) # probably Union{}
return Any
Expand Down Expand Up @@ -2448,7 +2447,9 @@ function abstract_eval(@nospecialize(e), vtypes::VarTable, sv::InferenceState)
n = sym.args[1]
if 1 <= n <= length(sv.sp)
val = sv.sp[n]
t = Const(true)
if !isa(val, TypeVar)
t = Const(true)
end
end
end
else
Expand Down Expand Up @@ -3837,9 +3838,13 @@ function effect_free(@nospecialize(e), src::CodeInfo, mod::Module, allow_volatil
elseif isa(e, Expr)
e = e::Expr
head = e.head
if head === :static_parameter || is_meta_expr_head(head)
if is_meta_expr_head(head)
return true
end
if head === :static_parameter
# if we aren't certain about the type, it might be an UndefVarError at runtime
return isa(e.typ, DataType) && isleaftype(e.typ)
end
if e.typ === Bottom
return false
end
Expand Down Expand Up @@ -4248,8 +4253,8 @@ function inlineable(@nospecialize(f), @nospecialize(ft), e::Expr, atypes::Vector
else
invoke_data = invoke_data::InvokeData
method = invoke_data.entry.func
(metharg, methsp) = ccall(:jl_match_method, Ref{SimpleVector}, (Any, Any),
atype_unlimited, method.sig)
(metharg, methsp) = ccall(:jl_match_method, Any, (Any, Any),
atype_unlimited, method.sig)::SimpleVector
methsp = methsp::SimpleVector
end

Expand Down Expand Up @@ -4983,20 +4988,21 @@ function inlining_pass(e::Expr, sv::InferenceState, stmts, ins)
aarg = e.args[i]
argt = exprtype(aarg, sv.src, sv.mod)
t = widenconst(argt)
if isa(aarg,Expr) && (is_known_call(aarg, tuple, sv.src, sv.mod) || is_known_call(aarg, svec, sv.src, sv.mod))
# apply(f,tuple(x,y,...)) => f(x,y,...)
newargs[i-2] = aarg.args[2:end]
elseif isa(argt,Const) && (isa(argt.val, Tuple) || isa(argt.val, SimpleVector)) &&
if isa(aarg, Expr) && (is_known_call(aarg, tuple, sv.src, sv.mod) || is_known_call(aarg, svec, sv.src, sv.mod))
# apply(f, tuple(x, y, ...)) => f(x, y, ...)
newargs[i - 2] = aarg.args[2:end]
elseif isa(argt, Const) && (isa(argt.val, Tuple) || isa(argt.val, SimpleVector)) &&
effect_free(aarg, sv.src, sv.mod, true)
newargs[i-2] = Any[ QuoteNode(x) for x in argt.val ]
val = argt.val
newargs[i - 2] = Any[ QuoteNode(val[i]) for i in 1:(length(val)::Int) ] # avoid making a tuple Generator here!
elseif isa(aarg, Tuple) || (isa(aarg, QuoteNode) && (isa(aarg.value, Tuple) || isa(aarg.value, SimpleVector)))
if isa(aarg, QuoteNode)
aarg = aarg.value
end
newargs[i-2] = Any[ QuoteNode(x) for x in aarg ]
newargs[i - 2] = Any[ QuoteNode(aarg[i]) for i in 1:(length(aarg)::Int) ] # avoid making a tuple Generator here!
elseif isa(t, DataType) && t.name === Tuple.name && !isvatuple(t) &&
length(t.parameters) <= sv.params.MAX_TUPLE_SPLAT
for k = (effect_free_upto+1):(i-3)
for k = (effect_free_upto + 1):(i - 3)
as = newargs[k]
for kk = 1:length(as)
ak = as[kk]
Expand All @@ -5007,7 +5013,7 @@ function inlining_pass(e::Expr, sv::InferenceState, stmts, ins)
end
end
end
effect_free_upto = i-3
effect_free_upto = i - 3
if effect_free(aarg, sv.src, sv.mod, true)
# apply(f,t::(x,y)) => f(t[1],t[2])
tmpv = aarg
Expand All @@ -5021,13 +5027,13 @@ function inlining_pass(e::Expr, sv::InferenceState, stmts, ins)
else
tp = t.parameters
end
newargs[i-2] = Any[ mk_getfield(tmpv,j,tp[j]) for j=1:length(tp) ]
newargs[i - 2] = Any[ mk_getfield(tmpv, j, tp[j]) for j in 1:(length(tp)::Int) ]
else
# not all args expandable
return e
end
end
splice!(stmts, ins:ins-1, newstmts)
splice!(stmts, ins:(ins - 1), newstmts)
ins += length(newstmts)
e.args = [Any[e.args[2]]; newargs...]

Expand Down Expand Up @@ -5155,11 +5161,17 @@ normvar(@nospecialize(s)) = s

# given a single-assigned var and its initializer `init`, return what we can
# replace `var` with, or `var` itself if we shouldn't replace it
function get_replacement(table, var::Union{SlotNumber, SSAValue}, @nospecialize(init), nargs, slottypes, ssavaluetypes)
function get_replacement(table::ObjectIdDict, var::Union{SlotNumber, SSAValue}, @nospecialize(init),
nargs::Int, slottypes::Vector{Any}, ssavaluetypes::Vector{Any})
#if isa(init, QuoteNode) # this can cause slight code size increases
# return init
if (isa(init, Expr) && init.head === :static_parameter) || isa(init, corenumtype) ||
init === () || init === nothing
if isa(init, Expr) && init.head === :static_parameter
# if we aren't certain about the type, it might be an UndefVarError at runtime (!effect_free)
# so we need to preserve the original point of assignment
if isa(init.typ, DataType) && isleaftype(init.typ)
return init
end
elseif isa(init, corenumtype) || init === () || init === nothing
return init
elseif isa(init, Slot) && is_argument(nargs, init::Slot)
# the transformation is not ideal if the assignment
Expand Down Expand Up @@ -5206,7 +5218,7 @@ function remove_redundant_temp_vars!(src::CodeInfo, nargs::Int, sa::ObjectIdDict
repls = ObjectIdDict()
for (v, init) in sa
repl = get_replacement(sa, v, init, nargs, slottypes, ssavaluetypes)
compare = isa(repl,TypedSlot) ? normslot(repl) : repl
compare = isa(repl, TypedSlot) ? normslot(repl) : repl
if compare !== v
repls[v] = repl
end
Expand Down
4 changes: 2 additions & 2 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,8 @@ function _dump_function(@nospecialize(f), @nospecialize(t), native::Bool, wrappe
ft = isa(f, Type) ? Type{f} : typeof(f)
tt = Tuple{ft, t.parameters...}
(ti, env) = ccall(:jl_match_method, Any, (Any, Any), tt, meth.sig)::SimpleVector
meth = func_for_method_checked(meth, tt)
linfo = ccall(:jl_specializations_get_linfo, Ref{Core.MethodInstance}, (Any, Any, Any, UInt), meth, tt, env, world)
meth = func_for_method_checked(meth, ti)
linfo = ccall(:jl_specializations_get_linfo, Ref{Core.MethodInstance}, (Any, Any, Any, UInt), meth, ti, env, world)
# get the code for it
return _dump_function_linfo(linfo, world, native, wrapper, strip_ir_metadata, dump_module, syntax, optimize, params)
end
Expand Down
103 changes: 102 additions & 1 deletion base/test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export @test, @test_throws, @test_broken, @test_skip, @test_warn, @test_nowarn
export @testset
# Legacy approximate testing functions, yet to be included
export @inferred
export detect_ambiguities
export detect_ambiguities, detect_unbound_args
export GenericString, GenericSet, GenericDict, GenericArray

#-----------------------------------------------------------------------
Expand Down Expand Up @@ -1257,6 +1257,107 @@ function detect_ambiguities(mods...;
return collect(ambs)
end

"""
detect_unbound_args(mod1, mod2...; imported=false)
Returns a vector of `Method`s which may have unbound type parameters.
Use `imported=true` if you wish to also test functions that were
imported into these modules from elsewhere.
Use `recursive=true` to test in all submodules.
"""
function detect_unbound_args(mods...;
imported::Bool = false,
recursive::Bool = false)
ambs = Set{Method}()
for mod in mods
for n in names(mod, true, imported)
Base.isdeprecated(mod, n) && continue
if !isdefined(mod, n)
println("Skipping ", mod, '.', n) # typically stale exports
continue
end
f = Base.unwrap_unionall(getfield(mod, n))
if recursive && isa(f, Module) && module_parent(f) === mod && module_name(f) === n
subambs = detect_unbound_args(f, imported=imported, recursive=recursive)
union!(ambs, subambs)
elseif isa(f, DataType) && isdefined(f.name, :mt)
mt = Base.MethodList(f.name.mt)
for m in mt
if has_unbound_vars(m.sig)
tuple_sig = Base.unwrap_unionall(m.sig)::DataType
if Base.isvatuple(tuple_sig)
params = tuple_sig.parameters[1:(end - 1)]
tuple_sig = Base.rewrap_unionall(Tuple{params...}, m.sig)
mf = ccall(:jl_gf_invoke_lookup, Any, (Any, UInt), tuple_sig, typemax(UInt))
if mf != nothing && mf.func !== m && mf.func.sig <: tuple_sig
continue
end
end
push!(ambs, m)
end
end
end
end
end
return collect(ambs)
end

# find if var will be constrained to have a definite value
# in any concrete leaftype subtype of typ
function constrains_param(var::TypeVar, @nospecialize(typ), cov::Bool)
typ === var && return true
while typ isa UnionAll
cov && constrains_param(var, typ.var.ub, cov) && return true
# typ.var.lb doesn't constrain var
typ = typ.body
end
if typ isa Union
# for unions, verify that both options would constrain var
ba = constrains_param(var, typ.a, cov)
bb = constrains_param(var, typ.b, cov)
(ba && bb) && return true
elseif typ isa DataType
# return true if any param constrains var
fc = length(typ.parameters)
if fc > 0
if typ.name === Tuple.name
# vararg tuple needs special handling
for i in 1:(fc - 1)
p = typ.parameters[i]
constrains_param(var, p, cov) && return true
end
lastp = typ.parameters[fc]
vararg = Base.unwrap_unionall(lastp)
if vararg isa DataType && vararg.name === Base._va_typename
N = vararg.parameters[2]
constrains_param(var, N, cov) && return true
# T = vararg.parameters[1] doesn't constrain var
else
constrains_param(var, lastp, cov) && return true
end
else
for i in 1:fc
p = typ.parameters[i]
constrains_param(var, p, false) && return true
end
end
end
end
return false
end

function has_unbound_vars(@nospecialize sig)
while sig isa UnionAll
var = sig.var
sig = sig.body
if !constrains_param(var, sig, true)
return true
end
end
return false
end


"""
The `GenericString` can be used to test generic string APIs that program to
the `AbstractString` interface, in order to ensure that functions can work
Expand Down
1 change: 1 addition & 0 deletions base/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ first(t::Tuple) = t[1]
# eltype

eltype(::Type{Tuple{}}) = Bottom
#eltype(::Type{Tuple{Vararg{E}}}) where {E} = E
eltype(::Type{<:Tuple{Vararg{E}}}) where {E} = E
function eltype(t::Type{<:Tuple})
@_pure_meta
Expand Down
3 changes: 2 additions & 1 deletion src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,8 @@ jl_value_t *jl_parse_eval_all(const char *fname,
jl_get_ptls_states()->world_age = jl_world_counter;
form = scm_to_julia(fl_ctx, expansion, 0);
jl_sym_t *head = NULL;
if (jl_is_expr(form)) head = ((jl_expr_t*)form)->head;
if (jl_is_expr(form))
head = ((jl_expr_t*)form)->head;
JL_SIGATOMIC_END();
jl_get_ptls_states()->world_age = jl_world_counter;
if (head == jl_incomplete_sym)
Expand Down
Loading

0 comments on commit d0381be

Please sign in to comment.