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.

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 type instability of closures capturing types (2) #40985

Merged
merged 5 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,21 @@ end
Expr(@nospecialize args...) = _expr(args...)

_is_internal(__module__) = __module__ === Core
# can be used in place of `@assume_effects :total` (supposed to be used for bootstrapping)
macro _total_meta()
return _is_internal(__module__) && Expr(:meta, Expr(:purity,
#=:consistent=#true,
#=:effect_free=#true,
#=:nothrow=#true,
#=:terminates_globally=#true,
#=:terminates_locally=#false,
#=:notaskstate=#true,
#=:inaccessiblememonly=#true,
#=:noub=#true,
#=:noub_if_noinbounds=#false,
#=:consistent_overlay=#false,
#=:nortcall=#true))
end
# can be used in place of `@assume_effects :foldable` (supposed to be used for bootstrapping)
macro _foldable_meta()
return _is_internal(__module__) && Expr(:meta, Expr(:purity,
Expand Down Expand Up @@ -310,6 +325,11 @@ convert(::Type{T}, x::T) where {T} = x
cconvert(::Type{T}, x) where {T} = convert(T, x)
unsafe_convert(::Type{T}, x::T) where {T} = x

# will be inserted by the frontend for closures
_typeof_captured_variable(@nospecialize t) = (@_total_meta; t isa Type && has_free_typevars(t) ? typeof(t) : Typeof(t))

has_free_typevars(@nospecialize t) = (@_total_meta; ccall(:jl_has_free_typevars, Int32, (Any,), t) === Int32(1))

# dispatch token indicating a kwarg (keyword sorter) call
function kwcall end
# deprecated internal functions:
Expand Down
3 changes: 2 additions & 1 deletion base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,8 @@ end

iskindtype(@nospecialize t) = (t === DataType || t === UnionAll || t === Union || t === typeof(Bottom))
isconcretedispatch(@nospecialize t) = isconcretetype(t) && !iskindtype(t)
has_free_typevars(@nospecialize(t)) = (@_total_meta; ccall(:jl_has_free_typevars, Cint, (Any,), t) != 0)
simeonschaub marked this conversation as resolved.
Show resolved Hide resolved

using Core: has_free_typevars

# equivalent to isa(v, Type) && isdispatchtuple(Tuple{v}) || v === Union{}
# and is thus perhaps most similar to the old (pre-1.0) `isleaftype` query
Expand Down
2 changes: 1 addition & 1 deletion src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -4246,7 +4246,7 @@ f(x) = yt(x)
(filter identity (map (lambda (v ve)
(if (is-var-boxed? v lam)
#f
`(call (core typeof) ,ve)))
`(call (core _typeof_captured_variable) ,ve)))
capt-vars var-exprs)))))
`(new ,(if (null? P)
type-name
Expand Down
12 changes: 6 additions & 6 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -951,34 +951,34 @@ end
end

# issue 43104

_has_free_typevars(t) = ccall(:jl_has_free_typevars, Cint, (Any,), t) != 0
@inline isGoodType(@nospecialize x::Type) =
x !== Any && !(@noinline Base.has_free_typevars(x))
x !== Any && !(@noinline _has_free_typevars(x))
let # aggressive inlining of single, abstract method match
src = code_typed((Type, Any,)) do x, y
isGoodType(x), isGoodType(y)
end |> only |> first
# both callsites should be inlined
@test count(isinvoke(:has_free_typevars), src.code) == 2
@test count(isinvoke(:_has_free_typevars), src.code) == 2
# `isGoodType(y::Any)` isn't fully covered, so the fallback is a method error
@test count(iscall((src, Core.throw_methoderror)), src.code) == 1 # fallback method error
end

@inline isGoodType2(cnd, @nospecialize x::Type) =
x !== Any && !(@noinline (cnd ? Core.Compiler.isType : Base.has_free_typevars)(x))
x !== Any && !(@noinline (cnd ? Core.Compiler.isType : _has_free_typevars)(x))
let # aggressive inlining of single, abstract method match (with constant-prop'ed)
src = code_typed((Type, Any,)) do x, y
isGoodType2(true, x), isGoodType2(true, y)
end |> only |> first
# both callsite should be inlined with constant-prop'ed result
@test count(isinvoke(:isType), src.code) == 2
@test count(isinvoke(:has_free_typevars), src.code) == 0
@test count(isinvoke(:_has_free_typevars), src.code) == 0
# `isGoodType(y::Any)` isn't fully covered, thus a MethodError gets inserted
@test count(iscall((src, Core.throw_methoderror)), src.code) == 1 # fallback method error
end

@noinline function checkBadType!(@nospecialize x::Type)
if x === Any || Base.has_free_typevars(x)
if x === Any || _has_free_typevars(x)
println(x)
end
return nothing
Expand Down
28 changes: 28 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,34 @@ end
@test foo21900 == 10
@test bar21900 == 11

let f = g -> x -> g(x)
@test f(Int)(1.0) === 1
@test @inferred(f(Int)) isa Function
@test fieldtype(typeof(f(Int)), 1) === Type{Int}
@test @inferred(f(Rational{Int})) isa Function
@test fieldtype(typeof(f(Rational{Int})), 1) === Type{Rational{Int}}
@test_broken @inferred(f(Rational)) isa Function
@test fieldtype(typeof(f(Rational)), 1) === Type{Rational}
@test_broken @inferred(f(Rational{Core.TypeVar(:T)})) isa Function
@test fieldtype(typeof(f(Rational{Core.TypeVar(:T)})), 1) === DataType
end
let f() = (T = Rational{Core.TypeVar(:T)}; () -> T)
@test f() isa Function
@test Base.infer_return_type(f()) == DataType
@test fieldtype(typeof(f()), 1) === DataType
t = f()()
@test t isa DataType
@test t.name.wrapper == Rational
@test length(t.parameters) == 1
@test t.parameters[1] isa Core.TypeVar
end
aviatesk marked this conversation as resolved.
Show resolved Hide resolved
function issue23618(a::AbstractVector)
T = eltype(a)
b = Vector{T}()
return [Set{T}() for x in a]
end
@test Base.infer_return_type(issue23618, (Vector{Int},)) == Vector{Set{Int}}

# ? syntax
@test (true ? 1 : false ? 2 : 3) == 1

Expand Down