Skip to content

Commit

Permalink
add typemap filtering option for Union{}
Browse files Browse the repository at this point in the history
Based on the new morespecific rule for Union{} and method definitions of
the specific form `f(..., Type{Union{}}, Vararg)`. If a method
definition exists with that specific form, the intersection visitor will
ignore all intersections that have that as their only result, saving
significant effort when working with lookups involving `Type{<:T}`
(which usually ended up mostly ambiguous anyways).

Fixes: #33780

This pattern turns out to have still to been making package loading
slow. We could keep adding methods following the ambiguity pattern
#46000 for the couple specific
functions that need it (constructor, eltype, IteratorEltype,
IteratorSize, and maybe a couple others) so the internals can detect
those and optimize functions that have that method pair. But it seems
somewhat odd, convoluted, and non-obvious behavior there. Instead, this
breaks all ambiguities in which Union{} is present explicitly in favor
of the method with Union{}. This means that when computing method
matches, as soon as we see one method definition with Union{}, we can
record that the method is the only possible match for that slot.

This, in essence, permits creating a rule for dispatch that a TypeVar
lower bound must be strictly a supertype of Union{}, but this creates it
at the function level, instead of expecting the user to add it to every
TypeVar they use to define methods.

This also lets us improve the error message for these cases (generally
they should error to avoid polluting the inference result), since we can
be assured this method will be called, and not result in an ambiguous
MethodError instead!

Reverts the functional change of #46000
  • Loading branch information
vtjnash committed Apr 13, 2023
1 parent 1a0db80 commit 69a0b62
Show file tree
Hide file tree
Showing 17 changed files with 182 additions and 99 deletions.
2 changes: 1 addition & 1 deletion base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ UInt8
```
"""
eltype(::Type) = Any
eltype(::Type{Bottom}) = throw(ArgumentError("Union{} does not have elements"))
eltype(::Type{Bottom}, slurp...) = throw(ArgumentError("Union{} does not have elements"))
eltype(x) = eltype(typeof(x))
eltype(::Type{<:AbstractArray{E}}) where {E} = @isdefined(E) ? E : Any

Expand Down
20 changes: 11 additions & 9 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,17 @@ UnionAll(v::TypeVar, @nospecialize(t)) = ccall(:jl_type_unionall, Any, (Any, Any

const Vararg = ccall(:jl_toplevel_eval_in, Any, (Any, Any), Core, _expr(:new, TypeofVararg))

# let the compiler assume that calling Union{} as a constructor does not need
# to be considered ever (which comes up often as Type{<:T})
Union{}(a...) = throw(MethodError(Union{}, a))
# dispatch token indicating a kwarg (keyword sorter) call
function kwcall end
# deprecated internal functions:
kwfunc(@nospecialize(f)) = kwcall
kwftype(@nospecialize(t)) = typeof(kwcall)

# Let the compiler assume that calling Union{} as a constructor does not need
# to be considered ever (which comes up often as Type{<:T} inference, and
# occasionally in user code from eltype).
Union{}(a...) = throw(ArgumentError("cannot construct a value of type Union{} for return result"))
kwcall(kwargs, ::Type{Union{}}, a...) = Union{}(a...)

Expr(@nospecialize args...) = _expr(args...)

Expand Down Expand Up @@ -369,12 +377,6 @@ include(m::Module, fname::String) = ccall(:jl_load_, Any, (Any, Any), m, fname)

eval(m::Module, @nospecialize(e)) = ccall(:jl_toplevel_eval_in, Any, (Any, Any), m, e)

# dispatch token indicating a kwarg (keyword sorter) call
function kwcall end
# deprecated internal functions:
kwfunc(@nospecialize(f)) = kwcall
kwftype(@nospecialize(t)) = typeof(kwcall)

mutable struct Box
contents::Any
Box(@nospecialize(x)) = new(x)
Expand Down
6 changes: 3 additions & 3 deletions base/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ that you may be able to leverage; see the
"""
abstract type BroadcastStyle end

struct Unknown <: BroadcastStyle end
BroadcastStyle(::Type{Union{}}, slurp...) = Unknown() # ambiguity resolution

"""
`Broadcast.Style{C}()` defines a [`BroadcastStyle`](@ref) signaling through the type
parameter `C`. You can use this as an alternative to creating custom subtypes of `BroadcastStyle`,
Expand All @@ -45,9 +48,6 @@ struct Style{T} <: BroadcastStyle end

BroadcastStyle(::Type{<:Tuple}) = Style{Tuple}()

struct Unknown <: BroadcastStyle end
BroadcastStyle(::Type{Union{}}) = Unknown() # ambiguity resolution

"""
`Broadcast.AbstractArrayStyle{N} <: BroadcastStyle` is the abstract supertype for any style
associated with an `AbstractArray` type.
Expand Down
2 changes: 1 addition & 1 deletion base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ See also: [`round`](@ref), [`trunc`](@ref), [`oftype`](@ref), [`reinterpret`](@r
function convert end

# ensure this is never ambiguous, and therefore fast for lookup
convert(T::Type{Union{}}, x) = throw(MethodError(convert, (T, x)))
convert(T::Type{Union{}}, x...) = error(ArgumentError("cannot convert a value to Union{} for assignment"))

convert(::Type{Type}, x::Type) = x # the ssair optimizer is strongly dependent on this method existing to avoid over-specialization
# in the absence of inlining-enabled
Expand Down
8 changes: 4 additions & 4 deletions base/generator.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ Base.HasLength()
"""
IteratorSize(x) = IteratorSize(typeof(x))
IteratorSize(::Type) = HasLength() # HasLength is the default
IteratorSize(::Type{Union{}}, slurp...) = throw(ArgumentError("Union{} does not have elements"))
IteratorSize(::Type{Any}) = SizeUnknown()

IteratorSize(::Type{<:Tuple}) = HasLength()
IteratorSize(::Type{<:AbstractArray{<:Any,N}}) where {N} = HasShape{N}()
IteratorSize(::Type{Generator{I,F}}) where {I,F} = IteratorSize(I)

IteratorSize(::Type{Any}) = SizeUnknown()

haslength(iter) = IteratorSize(iter) isa Union{HasShape, HasLength}

abstract type IteratorEltype end
Expand Down Expand Up @@ -126,7 +126,7 @@ Base.HasEltype()
"""
IteratorEltype(x) = IteratorEltype(typeof(x))
IteratorEltype(::Type) = HasEltype() # HasEltype is the default
IteratorEltype(::Type{Union{}}, slurp...) = throw(ArgumentError("Union{} does not have elements"))
IteratorEltype(::Type{Any}) = EltypeUnknown()

IteratorEltype(::Type{Generator{I,T}}) where {I,T} = EltypeUnknown()

IteratorEltype(::Type{Any}) = EltypeUnknown()
2 changes: 1 addition & 1 deletion base/indices.jl
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ particular, [`eachindex`](@ref) creates an iterator whose type depends
on the setting of this trait.
"""
IndexStyle(A::AbstractArray) = IndexStyle(typeof(A))
IndexStyle(::Type{Union{}}) = IndexLinear()
IndexStyle(::Type{Union{}}, slurp...) = IndexLinear()
IndexStyle(::Type{<:AbstractArray}) = IndexCartesian()
IndexStyle(::Type{<:Array}) = IndexLinear()
IndexStyle(::Type{<:AbstractRange}) = IndexLinear()
Expand Down
2 changes: 1 addition & 1 deletion base/missing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ nonmissingtype(::Type{T}) where {T} = typesplit(T, Missing)
function nonmissingtype_checked(T::Type)
R = nonmissingtype(T)
R >: T && error("could not compute non-missing type")
R <: Union{} && error("cannot convert a value to missing for assignment")
return R
end

Expand Down Expand Up @@ -69,7 +70,6 @@ convert(::Type{T}, x::T) where {T>:Union{Missing, Nothing}} = x
convert(::Type{T}, x) where {T>:Missing} = convert(nonmissingtype_checked(T), x)
convert(::Type{T}, x) where {T>:Union{Missing, Nothing}} = convert(nonmissingtype_checked(nonnothingtype_checked(T)), x)


# Comparison operators
==(::Missing, ::Missing) = missing
==(::Missing, ::Any) = missing
Expand Down
6 changes: 6 additions & 0 deletions base/promotion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,12 @@ it for new types as appropriate.
function promote_rule end

promote_rule(::Type, ::Type) = Bottom
# avoid enumerating unrelated possibilities when presented with Type{<:T},
# in general accordance with the result also given by promote_type
promote_rule(::Type{Bottom}, slurp...) = Bottom
promote_rule(::Type{Bottom}, ::Type{Bottom}, slurp...) = Bottom
promote_rule(::Type{Bottom}, ::Type{T}, slurp...) where {T} = T
promote_rule(::Type{T}, ::Type{Bottom}, slurp...) where {T} = T

promote_result(::Type,::Type,::Type{T},::Type{S}) where {T,S} = (@inline; promote_type(T,S))
# If no promote_rule is defined, both directions give Bottom. In that
Expand Down
1 change: 1 addition & 0 deletions base/some.jl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ end
function nonnothingtype_checked(T::Type)
R = nonnothingtype(T)
R >: T && error("could not compute non-nothing type")
R <: Union{} && error("cannot convert a value to nothing for assignment")
return R
end

Expand Down
4 changes: 3 additions & 1 deletion base/traits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ OrderStyle(::Type{<:Real}) = Ordered()
OrderStyle(::Type{<:AbstractString}) = Ordered()
OrderStyle(::Type{Symbol}) = Ordered()
OrderStyle(::Type{<:Any}) = Unordered()
OrderStyle(::Type{Union{}}) = Ordered()
OrderStyle(::Type{Union{}}, slurp...) = Ordered()

# trait for objects that support arithmetic
abstract type ArithmeticStyle end
Expand All @@ -23,6 +23,7 @@ ArithmeticStyle(instance) = ArithmeticStyle(typeof(instance))
ArithmeticStyle(::Type{<:AbstractFloat}) = ArithmeticRounds()
ArithmeticStyle(::Type{<:Integer}) = ArithmeticWraps()
ArithmeticStyle(::Type{<:Any}) = ArithmeticUnknown()
ArithmeticStyle(::Type{Union{}}, slurp...) = ArithmeticUnknown()

# trait for objects that support ranges with regular step
"""
Expand Down Expand Up @@ -58,5 +59,6 @@ ranges with an element type which is a subtype of `Integer`.
abstract type RangeStepStyle end
struct RangeStepRegular <: RangeStepStyle end # range with regular step
struct RangeStepIrregular <: RangeStepStyle end # range with rounding error
RangeStepStyle(::Type{Union{}}, slurp...) = RangeStepIrregular()

RangeStepStyle(instance) = RangeStepStyle(typeof(instance))
12 changes: 9 additions & 3 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,8 @@ static int get_intersect_visitor(jl_typemap_entry_t *oldentry, struct typemap_in
// skip if no world has both active
// also be careful not to try to scan something from the current dump-reload though
return 1;
// don't need to consider other similar methods if this oldentry will always fully intersect with them and dominates all of them
typemap_slurp_search(oldentry, &closure->match);
jl_method_t *oldmethod = oldentry->func.method;
if (closure->match.issubty // e.g. jl_subtype(closure->newentry.sig, oldentry->sig)
&& jl_subtype(oldmethod->sig, (jl_value_t*)closure->newentry->sig)) { // e.g. jl_type_equal(closure->newentry->sig, oldentry->sig)
Expand All @@ -1472,7 +1474,7 @@ static jl_value_t *get_intersect_matches(jl_typemap_t *defs, jl_typemap_entry_t
else
va = NULL;
}
struct matches_env env = {{get_intersect_visitor, (jl_value_t*)type, va,
struct matches_env env = {{get_intersect_visitor, (jl_value_t*)type, va, /* .search_slurp = */ 0,
/* .ti = */ NULL, /* .env = */ jl_emptysvec, /* .issubty = */ 0},
/* .newentry = */ newentry, /* .shadowed */ NULL, /* .replaced */ NULL};
JL_GC_PUSH3(&env.match.env, &env.match.ti, &env.shadowed);
Expand Down Expand Up @@ -3149,6 +3151,7 @@ struct ml_matches_env {
int intersections;
size_t world;
int lim;
int include_ambiguous;
// results:
jl_value_t *t; // array of method matches
size_t min_valid;
Expand Down Expand Up @@ -3204,6 +3207,9 @@ static int ml_matches_visitor(jl_typemap_entry_t *ml, struct typemap_intersectio
return 0;
closure->lim--;
}
// don't need to consider other similar methods if this ml will always fully intersect with them and dominates all of them
if (!closure->include_ambiguous || closure->lim != -1)
typemap_slurp_search(ml, &closure->match);
closure->matc = make_method_match((jl_tupletype_t*)closure->match.ti,
closure->match.env, meth,
closure->match.issubty ? FULLY_COVERS : NOT_FULLY_COVERS);
Expand Down Expand Up @@ -3253,9 +3259,9 @@ static jl_value_t *ml_matches(jl_methtable_t *mt,
else
va = NULL;
}
struct ml_matches_env env = {{ml_matches_visitor, (jl_value_t*)type, va,
struct ml_matches_env env = {{ml_matches_visitor, (jl_value_t*)type, va, /* .search_slurp = */ 0,
/* .ti = */ NULL, /* .env = */ jl_emptysvec, /* .issubty = */ 0},
intersections, world, lim, /* .t = */ jl_an_empty_vec_any,
intersections, world, lim, include_ambiguous, /* .t = */ jl_an_empty_vec_any,
/* .min_valid = */ *min_valid, /* .max_valid = */ *max_valid, /* .matc = */ NULL};
struct jl_typemap_assoc search = {(jl_value_t*)type, world, jl_emptysvec, 1, ~(size_t)0};
jl_value_t *isect2 = NULL;
Expand Down
2 changes: 2 additions & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1459,12 +1459,14 @@ struct typemap_intersection_env {
jl_typemap_intersection_visitor_fptr const fptr; // fptr to call on a match
jl_value_t *const type; // type to match
jl_value_t *const va; // the tparam0 for the vararg in type, if applicable (or NULL)
size_t search_slurp;
// output values
jl_value_t *ti; // intersection type
jl_svec_t *env; // intersection env (initialize to null to perform intersection without an environment)
int issubty; // if `a <: b` is true in `intersect(a,b)`
};
int jl_typemap_intersection_visitor(jl_typemap_t *a, int offs, struct typemap_intersection_env *closure);
void typemap_slurp_search(jl_typemap_entry_t *ml, struct typemap_intersection_env *closure);

// -- simplevector.c -- //

Expand Down
26 changes: 20 additions & 6 deletions src/subtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -2299,20 +2299,34 @@ int jl_has_intersect_type_not_kind(jl_value_t *t)
t = jl_unwrap_unionall(t);
if (t == (jl_value_t*)jl_any_type)
return 1;
if (jl_is_uniontype(t)) {
assert(!jl_is_vararg(t));
if (jl_is_uniontype(t))
return jl_has_intersect_type_not_kind(((jl_uniontype_t*)t)->a) ||
jl_has_intersect_type_not_kind(((jl_uniontype_t*)t)->b);
}
if (jl_is_typevar(t)) {
if (jl_is_typevar(t))
return jl_has_intersect_type_not_kind(((jl_tvar_t*)t)->ub);
}
if (jl_is_datatype(t)) {
if (jl_is_datatype(t))
if (((jl_datatype_t*)t)->name == jl_type_typename)
return 1;
}
return 0;
}

// compute if DataType<:t || Union<:t || UnionAll<:t etc.
int jl_has_intersect_kind_not_type(jl_value_t *t)
{
t = jl_unwrap_unionall(t);
if (t == (jl_value_t*)jl_any_type || jl_is_kind(t))
return 1;
assert(!jl_is_vararg(t));
if (jl_is_uniontype(t))
return jl_has_intersect_kind_not_type(((jl_uniontype_t*)t)->a) ||
jl_has_intersect_kind_not_type(((jl_uniontype_t*)t)->b);
if (jl_is_typevar(t))
return jl_has_intersect_kind_not_type(((jl_tvar_t*)t)->ub);
return 0;
}


JL_DLLEXPORT int jl_isa(jl_value_t *x, jl_value_t *t)
{
if (jl_typeis(x,t) || t == (jl_value_t*)jl_any_type)
Expand Down
Loading

0 comments on commit 69a0b62

Please sign in to comment.