-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
inference: fix missing LimitedAccuracy
handling in conditional_changes
#40744
Conversation
…nges` 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(JuliaLang#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} ``` Well, it's hard to test this kind of intermediate state of inference, so I didn't include test cases this PR fixes. (maybe can we use JET as an inference test ?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to leave behind a comment about why this is legal though? The ⊑
was answering this query correctly, since the value newtyp
is indeed lower in the lattice than typ
, but we're using ⊑
as an approximation to taking the intersection (tmeet
).
Co-authored-by: Jameson Nash <[email protected]>
@vtjnash bump |
oh, oops, I assumed you would be able to merge |
hehe, I don't have the write access yet. |
…nges` (JuliaLang#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(JuliaLang#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} ```
…nges` (JuliaLang#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(JuliaLang#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} ```
The following JET analysis told me that we missed to handle
LimitedAccuracy
inconditional_changes
and limited conditionalconstraints weren't propagated:
After this PR:
Well, it's hard to test this kind of intermediate state of inference, so
I didn't include test cases this PR fixes.
(maybe can we use JET as an inference test ?)