Skip to content

Commit 80f3a01

Browse files
committed
inference: fix too conservative effects for recursive cycles
The `:terminates` effect bit must be conservatively tainted unless recursion cycle has been fully resolved. As for other effects, there's no need to taint them at this moment because they will be tainted as we try to resolve the cycle. - fixes #52938 - xref #51092
1 parent 5034e87 commit 80f3a01

File tree

3 files changed

+22
-8
lines changed

3 files changed

+22
-8
lines changed

base/compiler/typeinfer.jl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,8 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
864864
update_valid_age!(caller, frame.valid_worlds)
865865
isinferred = is_inferred(frame)
866866
edge = isinferred ? mi : nothing
867-
effects = isinferred ? frame.result.ipo_effects : adjust_effects(Effects(), method) # effects are adjusted already within `finish` for ipo_effects
867+
effects = isinferred ? frame.result.ipo_effects : # effects are adjusted already within `finish` for ipo_effects
868+
adjust_effects(effects_for_cycle(frame.ipo_effects), method)
868869
exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
869870
# propagate newly inferred source to the inliner, allowing efficient inlining w/o deserialization:
870871
# note that this result is cached globally exclusively, so we can use this local result destructively
@@ -877,11 +878,16 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
877878
# return the current knowledge about this cycle
878879
frame = frame::InferenceState
879880
update_valid_age!(caller, frame.valid_worlds)
880-
effects = adjust_effects(Effects(), method)
881+
effects = adjust_effects(effects_for_cycle(frame.ipo_effects), method)
881882
exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
882883
return EdgeCallResult(frame.bestguess, exc_bestguess, nothing, effects)
883884
end
884885

886+
# The `:terminates` effect bit must be conservatively tainted unless recursion cycle has
887+
# been fully resolved. As for other effects, there's no need to taint them at this moment
888+
# because they will be tainted as we try to resolve the cycle.
889+
effects_for_cycle(effects::Effects) = Effects(effects; terminates=false)
890+
885891
function cached_return_type(code::CodeInstance)
886892
rettype = code.rettype
887893
isdefined(code, :rettype_const) || return rettype

test/compiler/AbstractInterpreter.jl

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,18 @@ function CC.concrete_eval_eligible(interp::Issue48097Interp,
152152
end
153153
@overlay Issue48097MT @noinline Core.throw_inexacterror(f::Symbol, ::Type{T}, val) where {T} = return
154154
issue48097(; kwargs...) = return 42
155-
@test_broken fully_eliminated(; interp=Issue48097Interp(), retval=42) do
155+
@test fully_eliminated(; interp=Issue48097Interp(), retval=42) do
156156
issue48097(; a=1f0, b=1.0)
157157
end
158158

159+
# https://github.com/JuliaLang/julia/issues/52938
160+
@newinterp Issue52938Interp
161+
@MethodTable ISSUE_52938_MT
162+
CC.method_table(interp::Issue52938Interp) = CC.OverlayMethodTable(CC.get_inference_world(interp), ISSUE_52938_MT)
163+
inner52938(x, types::Type, args...; kwargs...) = x
164+
outer52938(x) = @inline inner52938(x, Tuple{}; foo=Ref(42), bar=1)
165+
@test fully_eliminated(outer52938, (Any,); interp=Issue52938Interp(), retval=Argument(2))
166+
159167
# Should not concrete-eval overlayed methods in semi-concrete interpretation
160168
@newinterp OverlaySinInterp
161169
@MethodTable OverlaySinMT

test/compiler/effects.jl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,13 @@ Base.@assume_effects :terminates_globally function recur_termination1(x)
8989
0 x < 20 || error("bad fact")
9090
return x * recur_termination1(x-1)
9191
end
92-
@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,)))
92+
@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,)))
9393
@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination1, (Int,)))
9494
function recur_termination2()
9595
Base.@assume_effects :total !:terminates_globally
9696
recur_termination1(12)
9797
end
98-
@test_broken fully_eliminated(recur_termination2)
98+
@test fully_eliminated(recur_termination2)
9999
@test fully_eliminated() do; recur_termination2(); end
100100

101101
Base.@assume_effects :terminates_globally function recur_termination21(x)
@@ -104,15 +104,15 @@ Base.@assume_effects :terminates_globally function recur_termination21(x)
104104
return recur_termination22(x)
105105
end
106106
recur_termination22(x) = x * recur_termination21(x-1)
107-
@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,)))
108-
@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,)))
107+
@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,)))
108+
@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,)))
109109
@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination21, (Int,)))
110110
@test Core.Compiler.is_terminates(Base.infer_effects(recur_termination22, (Int,)))
111111
function recur_termination2x()
112112
Base.@assume_effects :total !:terminates_globally
113113
recur_termination21(12) + recur_termination22(12)
114114
end
115-
@test_broken fully_eliminated(recur_termination2x)
115+
@test fully_eliminated(recur_termination2x)
116116
@test fully_eliminated() do; recur_termination2x(); end
117117

118118
# anonymous function support for `@assume_effects`

0 commit comments

Comments
 (0)