From dc344285d5be2bfdf4ead01effa95643b7babc8b Mon Sep 17 00:00:00 2001 From: Simeon David Schaub Date: Sat, 12 Oct 2024 07:07:20 +0200 Subject: [PATCH] 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 Co-authored-by: Shuhei Kadowaki --- base/boot.jl | 20 ++++++++++++++++++++ base/reflection.jl | 3 ++- src/julia-syntax.scm | 2 +- test/compiler/inline.jl | 12 ++++++------ test/core.jl | 28 ++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 8 deletions(-) diff --git a/base/boot.jl b/base/boot.jl index 608e273d4b514..861c83a2edac5 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -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, @@ -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: diff --git a/base/reflection.jl b/base/reflection.jl index 80eeb4c4efb12..49d640ea40bab 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -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) + +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 diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index f1acb9c3250e1..4b3e6ae96898b 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -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 diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl index 2de6d9950d4e4..99fed00a7269d 100644 --- a/test/compiler/inline.jl +++ b/test/compiler/inline.jl @@ -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 diff --git a/test/core.jl b/test/core.jl index b27832209a835..5ba0e99e730d4 100644 --- a/test/core.jl +++ b/test/core.jl @@ -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 +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