Skip to content

Commit

Permalink
inference: fix missing LimitedAccuracy handling in `conditional_cha…
Browse files Browse the repository at this point in the history
…nges` (#40744)

The following JET analysis told me that we missed to handle
`LimitedAccuracy` in `conditional_changes` and limited conditional
constraints weren't propagated:
```julia
julia> using JET, Test

julia> report_call() do
           @testset "foo" begin; true; end
       end
┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1262 Test.finish(ts)
│┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1007 Test.print_test_results(ts)
││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:941 #self#(ts, 0)
│││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:961 Test.get_alignment(ts, 0)
││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1034 Test.map(#36, Base.getproperty(ts, :results))
│││││┌ @ abstractarray.jl:2362 Base.collect_similar(A, Base.Generator(f, A))
││││││┌ @ array.jl:638 Base._collect(cont, itr, Base.IteratorEltype(itr), Base.IteratorSize(itr))
│││││││┌ @ array.jl:723 Base.iterate(itr)
││││││││┌ @ generator.jl:47 Base.getproperty(g, :f)(Base.getindex(y, 1))
│││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1034 Test.get_alignment(t, Test.+(Core.getfield(#self#, :depth), 1))
│││││││││││││┌ @ array.jl:728 Base.collect_to_with_first!(Base._similar_for(c, Base.typeof(v1), itr, isz), v1, itr, st)
││││││││││││││┌ @ array.jl:734 Base.collect_to!(dest, itr, Base.+(i1, 1), st)
│││││││││││││││┌ @ /Users/aviatesk/julia/julia/base/array.jl:764 Base.collect_to!(new, itr, Base.+(i, 1), st)
││││││││││││││││┌ @ /Users/aviatesk/julia/julia/base/array.jl:758 Base.indexed_iterate(y, 2, _8)
│││││││││││││││││┌ @ tuple.jl:94 Base.iterate(I, state)
││││││││││││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing, Int64}): Base.iterate(I::Nothing, state::Int64)
│││││││││││││││││└───────────────
││││││││││││││││┌ @ /Users/aviatesk/julia/julia/base/array.jl:758 Base.indexed_iterate(y, 1)
│││││││││││││││││┌ @ tuple.jl:89 Base.iterate(I)
││││││││││││││││││ no matching method found for call signature (Tuple{typeof(iterate), Nothing}): Base.iterate(I::Nothing)
│││││││││││││││││└───────────────
Union{Test.FallbackTestSet, Test.DefaultTestSet}
```

After this PR:
```julia
julia> report_call() do
           @testset "foo" begin; true; end
       end
No errors !
Union{Test.FallbackTestSet, Test.DefaultTestSet}
```
  • Loading branch information
aviatesk authored May 13, 2021
1 parent feba24f commit 853a936
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
7 changes: 6 additions & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,12 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState)
end

function conditional_changes(changes::VarTable, @nospecialize(typ), var::Slot)
if typ (changes[slot_id(var)]::VarState).typ
oldtyp = (changes[slot_id(var)]::VarState).typ
# approximate test for `typ ∩ oldtyp` being better than `oldtyp`
# since we probably formed these types with `typesubstract`, the comparison is likely simple
if ignorelimited(typ) ignorelimited(oldtyp)
# typ is better unlimited, but we may still need to compute the tmeet with the limit "causes" since we ignored those in the comparison
oldtyp isa LimitedAccuracy && (typ = tmerge(typ, LimitedAccuracy(Bottom, oldtyp.causes)))
return StateUpdate(var, VarState(typ, false), changes, true)
end
return changes
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/typelattice.jl
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ struct StateUpdate
end

# Represent that the type estimate has been approximated, due to "causes"
# (only used in abstractinterpret, doesn't appear in optimize)
# (only used in abstract interpretion, doesn't appear in optimization)
# N.B. in the lattice, this is epsilon smaller than `typ` (except Union{})
struct LimitedAccuracy
typ
Expand Down

0 comments on commit 853a936

Please sign in to comment.