_register_kinds!: improve type stability when creating Vector

[nsajko]

Make the temporary existing_kinds::Vector infer concretely.

NB: a more thorough solution might do away with the construction of existing_kinds altogether, however this seems like an OK quick fix.

[nsajko]

See the PR that backports this PR to JuliaSyntax v1 for info on the Julia sysimage invalidations that are fixed.

[aviatesk]

Is this change really necessary when we merge #586 ?

[nsajko]

Either PR seems to fix the same sysimage invalidations. However both PRs improve the code_warntype look within the method, so I think it would be better to merge both?

[aviatesk]

I'm not opposed to adding the explicit type annotation, but wondering if #586 doesn't fix the type instability issue this PR is trying to fix, which may tell us another source of inference issues.

[nsajko]

No, #586 does not fix the type instability that is fixed by this PR. I checked with @eval JuliaSyntax and code_warntype before putting up the PRs.

[aviatesk]

What's the reproducer? Did you give concrete type for names argument type?

[nsajko]

Reproducer:

julia> using JuliaSyntax: _register_kinds!
Precompiling JuliaSyntax finished.
  1 dependency successfully precompiled in 27 seconds

julia> code_warntype(_register_kinds!, Tuple{Dict{Int, Union{Module, Symbol}}, Dict{UInt16, String}, Dict{String, UInt16}, Module, Int, Vector{String}})
MethodInstance for JuliaSyntax._register_kinds!(::Dict{Int64, Union{Module, Symbol}}, ::Dict{UInt16, String}, ::Dict{String, UInt16}, ::Module, ::Int64, ::Vector{String})
  from _register_kinds!(kind_modules, int_to_kindstr, kind_str_to_int, mod, module_id, names) @ JuliaSyntax ~/tmp/jl/JuliaSyntax.jl/src/julia/kinds.jl:78
Arguments
  #self#::Core.Const(JuliaSyntax._register_kinds!)
  kind_modules::Dict{Int64, Union{Module, Symbol}}
  int_to_kindstr::Dict{UInt16, String}
  kind_str_to_int::Dict{String, UInt16}
  mod::Module
  module_id::Int64
  names::Vector{String}
Locals
  #5::JuliaSyntax.var"#_register_kinds!##2#_register_kinds!##3"{Module}
  #4::JuliaSyntax.var"#_register_kinds!##0#_register_kinds!##1"{Dict{String, UInt16}}
  existing_kinds::Union{Vector{Nothing}, Vector{Union{Nothing, JuliaSyntax.Kind}}, Vector{JuliaSyntax.Kind}}
  m::Union{Module, Symbol}
Body::Nothing
1 ──       Core.NewvarNode(:(#5))
│          Core.NewvarNode(:(#4))
│          Core.NewvarNode(:(existing_kinds))
│          Core.NewvarNode(:(m))
│    %5  = JuliaSyntax.:>::Core.Const(>)
│    %6  = JuliaSyntax._kind_module_id_max::Core.Const(0x003f)
│    %7  = (%5)(module_id, %6)::Bool
└───       goto #3 if not %7
2 ── %9  = JuliaSyntax.error::Core.Const(error)
│    %10 = Base.string("Kind module id ", module_id, " is out of range")::String
│          (%9)(%10)
└───       Core.Const(:(goto %83))
3 ┄─ %13 = JuliaSyntax.:>=::Core.Const(>=)
│    %14 = JuliaSyntax.length::Core.Const(length)
│    %15 = (%14)(names)::Int64
│    %16 = JuliaSyntax.:<<::Core.Const(<<)
│    %17 = JuliaSyntax._kind_nbits::Core.Const(10)
│    %18 = (%16)(1, %17)::Core.Const(1024)
│    %19 = (%13)(%15, %18)::Bool
└───       goto #5 if not %19
4 ── %21 = JuliaSyntax.error::Core.Const(error)
│          (%21)("Too many kind names")
└───       Core.Const(:(goto %83))
5 ┄─ %24 = JuliaSyntax.:!::Core.Const(!)
│    %25 = JuliaSyntax.haskey::Core.Const(haskey)
│    %26 = (%25)(kind_modules, module_id)::Bool
│    %27 = (%24)(%26)::Bool
└───       goto #7 if not %27
6 ──       Base.setindex!(kind_modules, mod, module_id)
└───       goto #19
7 ──       (m = Base.getindex(kind_modules, module_id))
│    %32 = JuliaSyntax.:(==)::Core.Const(==)
│    %33 = m::Union{Module, Symbol}
│    %34 = JuliaSyntax.nameof::Core.Const(nameof)
│    %35 = (%34)(mod)::Symbol
│    %36 = (%32)(%33, %35)::Bool
└───       goto #9 if not %36
8 ──       Base.setindex!(kind_modules, mod, module_id)
└───       goto #19
9 ── %40 = JuliaSyntax.:(==)::Core.Const(==)
│    %41 = m::Union{Module, Symbol}
│    %42 = (%40)(%41, mod)::Bool
└───       goto #18 if not %42
10 ─ %44 = JuliaSyntax.:(var"#_register_kinds!##0#_register_kinds!##1")::Core.Const(JuliaSyntax.var"#_register_kinds!##0#_register_kinds!##1")
│    %45 = Core._typeof_captured_variable(kind_str_to_int)::Core.Const(Dict{String, UInt16})
│    %46 = Core.apply_type(%44, %45)::Core.Const(JuliaSyntax.var"#_register_kinds!##0#_register_kinds!##1"{Dict{String, UInt16}})
│          (#4 = %new(%46, kind_str_to_int))
│    %48 = #4::JuliaSyntax.var"#_register_kinds!##0#_register_kinds!##1"{Dict{String, UInt16}}
│    %49 = Base.Generator(%48, names)::Base.Generator{Vector{String}, JuliaSyntax.var"#_register_kinds!##0#_register_kinds!##1"{Dict{String, UInt16}}}
│          (existing_kinds = Base.collect(%49))
│    %51 = JuliaSyntax.any::Core.Const(any)
│    %52 = JuliaSyntax.isnothing::Core.Const(isnothing)
│    %53 = existing_kinds::Union{Vector{Nothing}, Vector{Union{Nothing, JuliaSyntax.Kind}}, Vector{JuliaSyntax.Kind}}
│    %54 = (%51)(%52, %53)::Bool
└───       goto #12 if not %54
11 ─       goto #15
12 ─ %57 = JuliaSyntax.:!::Core.Const(!)
│    %58 = JuliaSyntax.issorted::Core.Const(issorted)
│    %59 = existing_kinds::Union{Vector{Nothing}, Vector{Union{Nothing, JuliaSyntax.Kind}}, Vector{JuliaSyntax.Kind}}
│    %60 = (%58)(%59)::Bool
│    %61 = (%57)(%60)::Bool
└───       goto #14 if not %61
13 ─       goto #15
14 ─ %64 = JuliaSyntax.any::Core.Const(any)
│    %65 = JuliaSyntax.:(var"#_register_kinds!##2#_register_kinds!##3")::Core.Const(JuliaSyntax.var"#_register_kinds!##2#_register_kinds!##3")
│    %66 = Core._typeof_captured_variable(mod)::Core.Const(Module)
│    %67 = Core.apply_type(%65, %66)::Core.Const(JuliaSyntax.var"#_register_kinds!##2#_register_kinds!##3"{Module})
│          (#5 = %new(%67, mod))
│    %69 = #5::JuliaSyntax.var"#_register_kinds!##2#_register_kinds!##3"{Module}
│    %70 = existing_kinds::Union{Vector{Nothing}, Vector{Union{Nothing, JuliaSyntax.Kind}}, Vector{JuliaSyntax.Kind}}
│    %71 = (%64)(%69, %70)::Bool
└───       goto #16 if not %71
15 ┄ %73 = JuliaSyntax.error::Core.Const(error)
│    %74 = Base.string("Error registering kinds for module ", mod, " (register_kinds() called more than once inconsistently, or conflict with existing module kinds?)")::String
│          (%73)(%74)
└───       Core.Const(:(goto %78))
16 ┄       return nothing
17 ─       Core.Const(:(goto %83))
18 ┄ %79 = JuliaSyntax.error::Core.Const(error)
│    %80 = m::Union{Module, Symbol}
│    %81 = Base.string("Kind module ID ", module_id, " already claimed by module ", %80)::String
└───       (%79)(%81)
19 ┄ %83 = JuliaSyntax._register_kinds_names!::Core.Const(JuliaSyntax._register_kinds_names!)
│    %84 = (%83)(int_to_kindstr, kind_str_to_int, module_id, names)::Core.Const(nothing)
└───       return %84

See especially the line:

existing_kinds::Union{Vector{Nothing}, Vector{Union{Nothing, JuliaSyntax.Kind}}, Vector{JuliaSyntax.Kind}}

The signature for the code_warntype call should be the same as seen in some of the invalidations obtained by SnoopCompile.

[nsajko]

The above is with latest main, 00bd17e, which now has the merged PR #586.

[aviatesk]

The fact that the possibilities of Vector{Nothing} and Vector{JuliaSyntax.Kind}} are being considered is basically the compiler trying to perform more accurate type analysis, and since both are type stable types, I don't think they would become a source of invalidation. Of course, adding explicit types would be more compiler-friendly.