Skip to content

Commit 7466552

Browse files
authored
array: inline convert where possible (#56034)
This improves a common scenario, where someone wants to `push!` a poorly-typed object onto a well-typed Vector. For example: ```julia const NT = @NamedTuple{x::Int,y::Any} foo(v::Vector{NT}, x::Int, @nospecialize(y)) = push!(v, (; x, y)) ``` The `(; x, y)` is slightly poorly-typed here. It could have any type for its `.y` field before it is converted inside the `push!` to a NamedTuple with `y::Any` Without this PR, the dispatch for this `push!` cannot be inferred: ```julia julia> code_typed(foo, (Vector{NT}, Int, Any))[1] CodeInfo( 1 ─ ... │ %4 = %new(%3, x, y)::NamedTuple{(:x, :y), <:Tuple{Int64, Any}} │ %5 = Main.push!(v, %4)::Vector{@NamedTuple{x::Int64, y}} └── return %5 ) => Vector{@NamedTuple{x::Int64, y}} ``` With this PR, the above dynamic call is fully statically resolved and inlined (and therefore `--trim` compatible)
1 parent a007e80 commit 7466552

File tree

4 files changed

+48
-10
lines changed

4 files changed

+48
-10
lines changed

base/array.jl

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,13 @@ copyto!(dest::Array{T}, src::Array{T}) where {T} = copyto!(dest, 1, src, 1, leng
324324
# N.B: The generic definition in multidimensional.jl covers, this, this is just here
325325
# for bootstrapping purposes.
326326
function fill!(dest::Array{T}, x) where T
327-
xT = x isa T ? x : convert(T, x)::T
327+
@inline
328+
x = x isa T ? x : convert(T, x)::T
329+
return _fill!(dest, x)
330+
end
331+
function _fill!(dest::Array{T}, x::T) where T
328332
for i in eachindex(dest)
329-
@inbounds dest[i] = xT
333+
@inbounds dest[i] = x
330334
end
331335
return dest
332336
end
@@ -980,12 +984,22 @@ Dict{String, Int64} with 2 entries:
980984
function setindex! end
981985

982986
function setindex!(A::Array{T}, x, i::Int) where {T}
987+
@_propagate_inbounds_meta
988+
x = x isa T ? x : convert(T, x)::T
989+
return _setindex!(A, x, i)
990+
end
991+
function _setindex!(A::Array{T}, x::T, i::Int) where {T}
983992
@_noub_if_noinbounds_meta
984993
@boundscheck (i - 1)%UInt < length(A)%UInt || throw_boundserror(A, (i,))
985-
memoryrefset!(memoryrefnew(A.ref, i, false), x isa T ? x : convert(T,x)::T, :not_atomic, false)
994+
memoryrefset!(memoryrefnew(A.ref, i, false), x, :not_atomic, false)
986995
return A
987996
end
988997
function setindex!(A::Array{T}, x, i1::Int, i2::Int, I::Int...) where {T}
998+
@_propagate_inbounds_meta
999+
x = x isa T ? x : convert(T, x)::T
1000+
return _setindex!(A, x, i1, i2, I...)
1001+
end
1002+
function _setindex!(A::Array{T}, x, i1::Int, i2::Int, I::Int...) where {T}
9891003
@inline
9901004
@_noub_if_noinbounds_meta
9911005
@boundscheck checkbounds(A, i1, i2, I...) # generally _to_linear_index requires bounds checking
@@ -1267,10 +1281,16 @@ See also [`pushfirst!`](@ref).
12671281
function push! end
12681282

12691283
function push!(a::Vector{T}, item) where T
1284+
@inline
12701285
# convert first so we don't grow the array if the assignment won't work
1271-
itemT = item isa T ? item : convert(T, item)::T
1286+
# and also to avoid a dynamic dynamic dispatch in the common case that
1287+
# `item` is poorly-typed and `a` is well-typed
1288+
item = item isa T ? item : convert(T, item)::T
1289+
return _push!(a, item)
1290+
end
1291+
function _push!(a::Vector{T}, item::T) where T
12721292
_growend!(a, 1)
1273-
@_safeindex a[length(a)] = itemT
1293+
@_safeindex a[length(a)] = item
12741294
return a
12751295
end
12761296

@@ -1664,7 +1684,11 @@ julia> pushfirst!([1, 2, 3, 4], 5, 6)
16641684
```
16651685
"""
16661686
function pushfirst!(a::Vector{T}, item) where T
1687+
@inline
16671688
item = item isa T ? item : convert(T, item)::T
1689+
return _pushfirst!(a, item)
1690+
end
1691+
function _pushfirst!(a::Vector{T}, item::T) where T
16681692
_growbeg!(a, 1)
16691693
@_safeindex a[1] = item
16701694
return a
@@ -1750,12 +1774,16 @@ julia> insert!(Any[1:6;], 3, "here")
17501774
```
17511775
"""
17521776
function insert!(a::Array{T,1}, i::Integer, item) where T
1777+
@_propagate_inbounds_meta
1778+
item = item isa T ? item : convert(T, item)::T
1779+
return _insert!(a, i, item)
1780+
end
1781+
function _insert!(a::Array{T,1}, i::Integer, item::T) where T
17531782
@_noub_meta
17541783
# Throw convert error before changing the shape of the array
1755-
_item = item isa T ? item : convert(T, item)::T
17561784
_growat!(a, i, 1)
17571785
# :noub, because _growat! already did bound check
1758-
@inbounds a[i] = _item
1786+
@inbounds a[i] = item
17591787
return a
17601788
end
17611789

base/namedtuple.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,11 @@ nextind(@nospecialize(t::NamedTuple), i::Integer) = Int(i)+1
179179

180180
convert(::Type{NT}, nt::NT) where {names, NT<:NamedTuple{names}} = nt
181181
convert(::Type{NT}, nt::NT) where {names, T<:Tuple, NT<:NamedTuple{names,T}} = nt
182-
convert(::Type{NT}, t::Tuple) where {NT<:NamedTuple} = NT(t)
182+
convert(::Type{NT}, t::Tuple) where {NT<:NamedTuple} = (@inline NT(t))::NT
183183

184184
function convert(::Type{NamedTuple{names,T}}, nt::NamedTuple{names}) where {names,T<:Tuple}
185-
NamedTuple{names,T}(T(nt))::NamedTuple{names,T}
185+
NT = NamedTuple{names,T}
186+
(@inline NT(nt))::NT
186187
end
187188

188189
function convert(::Type{NT}, nt::NamedTuple{names}) where {names, NT<:NamedTuple{names}}

test/abstractarray.jl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
using Random, LinearAlgebra
44

5+
include("compiler/irutils.jl")
6+
57
isdefined(Main, :InfiniteArrays) || @eval Main include("testhelpers/InfiniteArrays.jl")
68
using .Main.InfiniteArrays
79

@@ -1403,6 +1405,8 @@ end
14031405
Base.push!(tpa::TestPushArray{T}, a::T) where T = push!(tpa.data, a)
14041406
Base.pushfirst!(tpa::TestPushArray{T}, a::T) where T = pushfirst!(tpa.data, a)
14051407

1408+
push_slightly_abstract_namedtuple(v::Vector{@NamedTuple{x::Int,y::Any}}, x::Int, @nospecialize(y)) = push!(v, (; x, y))
1409+
14061410
@testset "push! and pushfirst!" begin
14071411
a_orig = [1]
14081412
tpa = TestPushArray{Int, 2}(a_orig)
@@ -1412,6 +1416,11 @@ Base.pushfirst!(tpa::TestPushArray{T}, a::T) where T = pushfirst!(tpa.data, a)
14121416
tpa = TestPushArray{Int, 2}(a_orig)
14131417
pushfirst!(tpa, 6, 5, 4, 3, 2)
14141418
@test tpa.data == reverse(collect(1:6))
1419+
1420+
let src = code_typed1(push_slightly_abstract_namedtuple, (Vector{@NamedTuple{x::Int,y::Any}},Int,Any))
1421+
# After optimization, all `push!` and `convert` calls should have been inlined
1422+
@test all((x)->!iscall((src, push!))(x) && !iscall((src, convert))(x), src.code)
1423+
end
14151424
end
14161425

14171426
mutable struct SimpleArray{T} <: AbstractVector{T}

test/compiler/effects.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,7 @@ end
12251225
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(getindex, (Vector{Int},Int)))
12261226
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(getindex, (Vector{Any},Int)))
12271227
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(setindex!, (Vector{Int},Int,Int)))
1228-
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(setindex!, (Vector{Any},Any,Int)))
1228+
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(Base._setindex!, (Vector{Any},Any,Int)))
12291229
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(isassigned, (Vector{Int},Int)))
12301230
@test Core.Compiler.is_noub_if_noinbounds(Base.infer_effects(isassigned, (Vector{Any},Int)))
12311231
@test Base.infer_effects((Vector{Int},Int)) do xs, i

0 commit comments

Comments
 (0)