Skip to content

Commit

Permalink
lattice: fix correctness bug in tmerge limiting (#50939)
Browse files Browse the repository at this point in the history
In 162ee48, the added code causes us to violate the tmerge_fast_path
requirements on the lattice. This was causing the fall-though from the
earlier tmerge_fast_path to not return correct answers to inference
anymore. Adding back another tmerge_fast_path, on the types, allows us
to recover correctness without regressing accuracy to before #47992.

Also added a test case for an example in which tmerge_fast_path does not
return a correctly limited answer, since it does not model UnionAll
complexity growth.
  • Loading branch information
vtjnash authored Aug 21, 2023
1 parent 74ce6cf commit 6fd82d7
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
6 changes: 3 additions & 3 deletions base/compiler/typelimits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ end
return tmerge(wl, typea, typeb)
end

@nospecializeinfer function tmerge(::JLTypeLattice, @nospecialize(typea::Type), @nospecialize(typeb::Type))
@nospecializeinfer function tmerge(lattice::JLTypeLattice, @nospecialize(typea::Type), @nospecialize(typeb::Type))
# it's always ok to form a Union of two concrete types
act = isconcretetype(typea)
bct = isconcretetype(typeb)
Expand All @@ -687,8 +687,8 @@ end
if (act || isType(typea)) && (bct || isType(typeb))
return Union{typea, typeb}
end
typea <: typeb && return typeb
typeb <: typea && return typea
u = tmerge_fast_path(lattice, typea, typeb)
u === nothing || return u
return tmerge_types_slow(typea, typeb)
end

Expand Down
13 changes: 13 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4584,6 +4584,19 @@ end
g = Base.ImmutableDict(g, 1=>2)
end
end |> only === Union{}

a = Val{Union{}}
a = Core.Compiler.tmerge(Union{a, Val{a}}, a)
@test a == Union{Val{Union{}}, Val{Val{Union{}}}}
a = Core.Compiler.tmerge(Union{a, Val{a}}, a)
@test a == Union{Val{Union{}}, Val{Val{Union{}}}, Val{Union{Val{Union{}}, Val{Val{Union{}}}}}}
a = Core.Compiler.tmerge(Union{a, Val{a}}, a)
@test a == Val

a = Val{Union{}}
a = Core.Compiler.tmerge(Core.Compiler.JLTypeLattice(), Val{<:a}, a)
@test_broken a != Val{<:Val{Union{}}}
@test_broken a == Val{<:Val} || a == Val
end

# Test that a function-wise `@max_methods` works as expected
Expand Down

0 comments on commit 6fd82d7

Please sign in to comment.