Unexpected method selected for dynamic dispatch on Julia 1.10.9

[Drvi]

The following code

struct NullableVector{T,V<:AbstractVector{T}} <: AbstractVector{Union{Nothing,T}}
    data::V
end
const NullableColumn{T} = NullableVector{T,Vector{T}}
const Column{T} = Union{Vector{T},NullableColumn{T}}

_maybe_push!(::Vector{T}) where {T} = sizeof(T)
_maybe_push!(column::NullableVector{T}) where {T} = _maybe_push!(column.data)
_maybe_push!(column::Column) = (println("Why we dispatch here? Input type is Vector type `$(typeof(column))`"); 0)

function _recycle!(columns::Vector{Column})
    for col in columns
        _maybe_push!(col)
    end
end

# Repro
cols = Column[Int[], UInt[], Float64[]]
_recycle!(cols)

Prints the following on Julia 1.10.9:

Why we dispatch here? Input type is Vector type `Vector{Int64}`
Why we dispatch here? Input type is Vector type `Vector{UInt64}`
Why we dispatch here? Input type is Vector type `Vector{Float64}`

I.e. we're dispatching to the _maybe_push!(column::Column) method, while the more specific _maybe_push!(::Vector{T}) method exists.

This does not reproduce if we @noinline the _maybe_push!(col) call site.
This issue doesn't reproduce on Julia 1.11.4 and 1.13.0-DEV.241, the expected method is selected.

[oscardssmith]

Is this new on 1.10.9/1.10? If not, this is just inference getting better in newer versions, so there's not much to do.

[christiangnrd]

I ran the MWE in a bunch of different Julia versions and only got the undesired output in 1.10 (I tried 1.10.0. and 1.10.9)

[nickrobinson251]

very slight reduction / extra weirdness:

struct NullableVector{T,V<:AbstractVector{T}} <: AbstractVector{Union{Nothing,T}}
    data::V
end
const NullableColumn{T} = NullableVector{T,Vector{T}}

_maybe_push!(::Vector) = 42
_maybe_push!(column::NullableVector) = _maybe_push!(column.data)
_maybe_push!(column::Union{Vector,NullableColumn}) = (println("Why we dispatch here? Input type is Vector type `$(typeof(column))`"); 0)

function _recycle!(columns)
    for col in columns
        _maybe_push!(col)
    end
end

# c1 is never used! but delete it and issue doesn't repro anymore
c1 = (Union{Vector{T},NullableColumn{T}} where T)[Int[], UInt[], Float64[]] 
c2 = Union{Vector,NullableColumn}[Int[], UInt[], Float64[]]

_recycle!(c2)

Why we dispatch here? Input type is Vector type `Vector{Int64}`
Why we dispatch here? Input type is Vector type `Vector{UInt64}`
Why we dispatch here? Input type is Vector type `Vector{Float64}`

[Drvi]

@oscardssmith Thanks for looking into it! I feel like this should have the "bug" label and not the "performance" label though, since this is a correctness issue, not a performance issue -- we are not selecting the most specific method for the given input and this can (and did) lead to unexpected errors.

Currently in _recycle! we inline the _maybe_push!(column::Column), while the correct thing, in my view, would be to not inline the _maybe_push! and dynamically dispatch to the most specific method for each element -- if anything I'm asking to make the perf worse:). This already happens when I annotate the call site with @noinline and it feels strange that the inlining decision would affect program logic, if that makes sense.

[oscardssmith]

Further reduced:

function _recycle2!(col)
     _maybe_push!(col[])
end
c1 = Ref{Union{Vector{T},NullableColumn{T}} where T}(Int[])
c2 = Ref{Union{Vector,NullableColumn}}(Int[])
 _recycle2!(c2)
Why we dispatch here? Input type is Vector type `Vector{Int64}`

[KristofferC]

Bisecting shows that 762801c fixed this. It backported trivially to 1.10. Will test locally if this is fixed with that commit.

Seems like it did fix it. The commit is on the backport branch and will be in next release.

[Drvi]

Well, that was quick! Thank you, @KristofferC and @oscardssmith!

[KristofferC]

What's the assignment for @oscardssmith?