Skip to content

Commit dc34428

Browse files
simeonschaubtkfaviatesk
authored
Fix type instability of closures capturing types (2) (#40985)
Instead of closures lowering to `typeof` for the types of captured fields, this introduces a new function `_typeof_captured_variable` that returns `Type{T}` if `T` is a type (w/o free typevars). - replaces/closes #35970 - fixes #23618 --------- Co-authored-by: Takafumi Arakaki <[email protected]> Co-authored-by: Shuhei Kadowaki <[email protected]>
1 parent cff9cca commit dc34428

File tree

5 files changed

+57
-8
lines changed

5 files changed

+57
-8
lines changed

base/boot.jl

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,21 @@ end
272272
Expr(@nospecialize args...) = _expr(args...)
273273

274274
_is_internal(__module__) = __module__ === Core
275+
# can be used in place of `@assume_effects :total` (supposed to be used for bootstrapping)
276+
macro _total_meta()
277+
return _is_internal(__module__) && Expr(:meta, Expr(:purity,
278+
#=:consistent=#true,
279+
#=:effect_free=#true,
280+
#=:nothrow=#true,
281+
#=:terminates_globally=#true,
282+
#=:terminates_locally=#false,
283+
#=:notaskstate=#true,
284+
#=:inaccessiblememonly=#true,
285+
#=:noub=#true,
286+
#=:noub_if_noinbounds=#false,
287+
#=:consistent_overlay=#false,
288+
#=:nortcall=#true))
289+
end
275290
# can be used in place of `@assume_effects :foldable` (supposed to be used for bootstrapping)
276291
macro _foldable_meta()
277292
return _is_internal(__module__) && Expr(:meta, Expr(:purity,
@@ -310,6 +325,11 @@ convert(::Type{T}, x::T) where {T} = x
310325
cconvert(::Type{T}, x) where {T} = convert(T, x)
311326
unsafe_convert(::Type{T}, x::T) where {T} = x
312327

328+
# will be inserted by the frontend for closures
329+
_typeof_captured_variable(@nospecialize t) = (@_total_meta; t isa Type && has_free_typevars(t) ? typeof(t) : Typeof(t))
330+
331+
has_free_typevars(@nospecialize t) = (@_total_meta; ccall(:jl_has_free_typevars, Int32, (Any,), t) === Int32(1))
332+
313333
# dispatch token indicating a kwarg (keyword sorter) call
314334
function kwcall end
315335
# deprecated internal functions:

base/reflection.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,8 @@ end
862862

863863
iskindtype(@nospecialize t) = (t === DataType || t === UnionAll || t === Union || t === typeof(Bottom))
864864
isconcretedispatch(@nospecialize t) = isconcretetype(t) && !iskindtype(t)
865-
has_free_typevars(@nospecialize(t)) = (@_total_meta; ccall(:jl_has_free_typevars, Cint, (Any,), t) != 0)
865+
866+
using Core: has_free_typevars
866867

867868
# equivalent to isa(v, Type) && isdispatchtuple(Tuple{v}) || v === Union{}
868869
# and is thus perhaps most similar to the old (pre-1.0) `isleaftype` query

src/julia-syntax.scm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4246,7 +4246,7 @@ f(x) = yt(x)
42464246
(filter identity (map (lambda (v ve)
42474247
(if (is-var-boxed? v lam)
42484248
#f
4249-
`(call (core typeof) ,ve)))
4249+
`(call (core _typeof_captured_variable) ,ve)))
42504250
capt-vars var-exprs)))))
42514251
`(new ,(if (null? P)
42524252
type-name

test/compiler/inline.jl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -951,34 +951,34 @@ end
951951
end
952952

953953
# issue 43104
954-
954+
_has_free_typevars(t) = ccall(:jl_has_free_typevars, Cint, (Any,), t) != 0
955955
@inline isGoodType(@nospecialize x::Type) =
956-
x !== Any && !(@noinline Base.has_free_typevars(x))
956+
x !== Any && !(@noinline _has_free_typevars(x))
957957
let # aggressive inlining of single, abstract method match
958958
src = code_typed((Type, Any,)) do x, y
959959
isGoodType(x), isGoodType(y)
960960
end |> only |> first
961961
# both callsites should be inlined
962-
@test count(isinvoke(:has_free_typevars), src.code) == 2
962+
@test count(isinvoke(:_has_free_typevars), src.code) == 2
963963
# `isGoodType(y::Any)` isn't fully covered, so the fallback is a method error
964964
@test count(iscall((src, Core.throw_methoderror)), src.code) == 1 # fallback method error
965965
end
966966

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

980980
@noinline function checkBadType!(@nospecialize x::Type)
981-
if x === Any || Base.has_free_typevars(x)
981+
if x === Any || _has_free_typevars(x)
982982
println(x)
983983
end
984984
return nothing

test/core.jl

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,34 @@ end
796796
@test foo21900 == 10
797797
@test bar21900 == 11
798798

799+
let f = g -> x -> g(x)
800+
@test f(Int)(1.0) === 1
801+
@test @inferred(f(Int)) isa Function
802+
@test fieldtype(typeof(f(Int)), 1) === Type{Int}
803+
@test @inferred(f(Rational{Int})) isa Function
804+
@test fieldtype(typeof(f(Rational{Int})), 1) === Type{Rational{Int}}
805+
@test_broken @inferred(f(Rational)) isa Function
806+
@test fieldtype(typeof(f(Rational)), 1) === Type{Rational}
807+
@test_broken @inferred(f(Rational{Core.TypeVar(:T)})) isa Function
808+
@test fieldtype(typeof(f(Rational{Core.TypeVar(:T)})), 1) === DataType
809+
end
810+
let f() = (T = Rational{Core.TypeVar(:T)}; () -> T)
811+
@test f() isa Function
812+
@test Base.infer_return_type(f()) == DataType
813+
@test fieldtype(typeof(f()), 1) === DataType
814+
t = f()()
815+
@test t isa DataType
816+
@test t.name.wrapper == Rational
817+
@test length(t.parameters) == 1
818+
@test t.parameters[1] isa Core.TypeVar
819+
end
820+
function issue23618(a::AbstractVector)
821+
T = eltype(a)
822+
b = Vector{T}()
823+
return [Set{T}() for x in a]
824+
end
825+
@test Base.infer_return_type(issue23618, (Vector{Int},)) == Vector{Set{Int}}
826+
799827
# ? syntax
800828
@test (true ? 1 : false ? 2 : 3) == 1
801829

0 commit comments

Comments
 (0)