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

base/compiler/typeinfer.jl

@@ -864,7 +864,8 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
         update_valid_age!(caller, frame.valid_worlds)
         isinferred = is_inferred(frame)
         edge = isinferred ? mi : nothing
-        effects = isinferred ? frame.result.ipo_effects : adjust_effects(Effects(), method) # effects are adjusted already within `finish` for ipo_effects
+        effects = isinferred ? frame.result.ipo_effects : # effects are adjusted already within `finish` for ipo_effects
+            adjust_effects(effects_for_cycle(frame.ipo_effects), method)
         exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
         # propagate newly inferred source to the inliner, allowing efficient inlining w/o deserialization:
         # note that this result is cached globally exclusively, so we can use this local result destructively
@@ -877,11 +878,18 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize
     # return the current knowledge about this cycle
     frame = frame::InferenceState
     update_valid_age!(caller, frame.valid_worlds)
-    effects = adjust_effects(Effects(), method)
+    effects = adjust_effects(effects_for_cycle(frame.ipo_effects), method)
     exc_bestguess = refine_exception_type(frame.exc_bestguess, effects)
     return EdgeCallResult(frame.bestguess, exc_bestguess, nothing, effects)
 end
 
+# The `:terminates` effect bit must be conservatively tainted unless recursion cycle has
+# been fully resolved. As for other effects, don't taint them at this moment because they
+# would be tainted as we try to resolve the cycle.
+# TODO Theoretically, this function is not legal and can potentially cause issues. Ideally
+# the effects should be implemented as cycle converging, similar to how rt and exct are.
+effects_for_cycle(effects::Effects) = Effects(effects; terminates=false)
+
 function cached_return_type(code::CodeInstance)
     rettype = code.rettype
     isdefined(code, :rettype_const) || return rettype

test/compiler/AbstractInterpreter.jl

@@ -152,10 +152,18 @@ function CC.concrete_eval_eligible(interp::Issue48097Interp,
 end
 @overlay Issue48097MT @noinline Core.throw_inexacterror(f::Symbol, ::Type{T}, val) where {T} = return
 issue48097(; kwargs...) = return 42
-@test_broken fully_eliminated(; interp=Issue48097Interp(), retval=42) do
+@test fully_eliminated(; interp=Issue48097Interp(), retval=42) do
     issue48097(; a=1f0, b=1.0)
 end
 
+# https://github.com/JuliaLang/julia/issues/52938
+@newinterp Issue52938Interp
+@MethodTable ISSUE_52938_MT
+CC.method_table(interp::Issue52938Interp) = CC.OverlayMethodTable(CC.get_inference_world(interp), ISSUE_52938_MT)
+inner52938(x, types::Type, args...; kwargs...) = x
+outer52938(x) = @inline inner52938(x, Tuple{}; foo=Ref(42), bar=1)
+@test fully_eliminated(outer52938, (Any,); interp=Issue52938Interp(), retval=Argument(2))
+
 # Should not concrete-eval overlayed methods in semi-concrete interpretation
 @newinterp OverlaySinInterp
 @MethodTable OverlaySinMT

test/compiler/effects.jl

@@ -89,13 +89,13 @@ Base.@assume_effects :terminates_globally function recur_termination1(x)
     0 ≤ x < 20 || error("bad fact")
     return x * recur_termination1(x-1)
 end
-@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,)))
+@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination1, (Int,)))
 @test Core.Compiler.is_terminates(Base.infer_effects(recur_termination1, (Int,)))
 function recur_termination2()
     Base.@assume_effects :total !:terminates_globally
     recur_termination1(12)
 end
-@test_broken fully_eliminated(recur_termination2)
+@test fully_eliminated(recur_termination2)
 @test fully_eliminated() do; recur_termination2(); end
 
 Base.@assume_effects :terminates_globally function recur_termination21(x)
@@ -104,15 +104,15 @@ Base.@assume_effects :terminates_globally function recur_termination21(x)
     return recur_termination22(x)
 end
 recur_termination22(x) = x * recur_termination21(x-1)
-@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,)))
-@test_broken Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,)))
+@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination21, (Int,)))
+@test Core.Compiler.is_foldable(Base.infer_effects(recur_termination22, (Int,)))
 @test Core.Compiler.is_terminates(Base.infer_effects(recur_termination21, (Int,)))
 @test Core.Compiler.is_terminates(Base.infer_effects(recur_termination22, (Int,)))
 function recur_termination2x()
     Base.@assume_effects :total !:terminates_globally
     recur_termination21(12) + recur_termination22(12)
 end
-@test_broken fully_eliminated(recur_termination2x)
+@test fully_eliminated(recur_termination2x)
 @test fully_eliminated() do; recur_termination2x(); end
 
 # anonymous function support for `@assume_effects`