JuliaLang / julia / 1508

78%

Build Type

push

buildkite

Committed by web-flow

Commit Message

compiler: Filter dominated original methods in overlay matching (#61581)

When merging overlay and base method match results in `findall(sig,
::OverlayMethodTable)`, base methods whose signature is fully covered by
any of matching overlay methods' signatures are now filtered out.

Previously, both the overlay and base methods with the same signature
appeared in the results. This caused type inference to union their
return types even though the overlay should take dispatch priority.

While this is a general inference correctness improvement that makes
inference's emulation of overlay method selection at runtime more
"correct", it might be worth explaining the concrete need that motivates
this change:

Currently, inference for code like `[(x::Vector{Vector{Int}})...;]`
unexpectedly infers `Union{Vector{Int},Vector{Any}}` instead of
`Vector{Int}`, causing type instability isseus as reported in
aviatesk/JET.jl#413.
The root cause of this type instability is the existence of the fallback
method `vcat() = Any[]`.
To fix this, I initially attempted to remove the `vcat()` method
(JuliaLang/julia#47628), but that turned out to be a considerably
breaking change, so I concluded it was not viable to proceed in that
direction as of Julia v1 time.
I also considered alternative approaches: 1.) changing the lowering of
`[x...;]` to produce a primitive for `vcat` that would allow inference
to work correctly in such cases, and 2.) adding a typed required 1st
argument to other `vcat` methods so that dispatch to `vcat()` would not
be considered — but both of those also proved impractical for various
reasons (primarily because the complexity they introduce is too high).
Removing the fallback method definitions for `vcat()` and `hcat()`
themselves would be the simplest and most elegant fix, but that would
have to wait for the release of Julia v2.

Therefore, rather than fixing this type instability on the Julia base
side, I believe the least invasive solution is to define,... (continued)

Coverage Stats

0 of 5 new or added lines in 1 file covered. (0.0%)

471 existing lines in 19 files now uncovered.

65259 of 83993 relevant lines covered (77.7%)

23119906.18 hits per line