From cda6010eb0c58389a64fafc2b5e4537998dbbc49 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Wed, 22 Mar 2023 01:55:35 -0400 Subject: [PATCH] effects: Do not over-taint :terminates in abstract cycle We already infer non-termination as part of the conservative effects we assume at the point in the call-graph that recursion is detected. As a result, it should be sound to allow this to propagate through the Effects system naturally rather than eagerly marking our callers as non-terminating. Doing this is important to avoid tainting non-termination from purely abstract cycles, where inference is forced to analyze methods that do not correspond to real calls at runtime, such as `Base._return_type`. Resolves #48983. --- base/compiler/typeinfer.jl | 4 ---- test/compiler/effects.jl | 11 +++++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl index 72f1b7b76c8fd..b7e213f3f2147 100644 --- a/base/compiler/typeinfer.jl +++ b/base/compiler/typeinfer.jl @@ -779,13 +779,9 @@ function merge_call_chain!(interp::AbstractInterpreter, parent::InferenceState, # then add all backedges of parent <- parent.parent # and merge all of the callers into ancestor.callers_in_cycle # and ensure that walking the parent list will get the same result (DAG) from everywhere - # Also taint the termination effect, because we can no longer guarantee the absence - # of recursion. - merge_effects!(interp, parent, Effects(EFFECTS_TOTAL; terminates=false)) while true add_cycle_backedge!(parent, child, parent.currpc) union_caller_cycle!(ancestor, child) - merge_effects!(interp, child, Effects(EFFECTS_TOTAL; terminates=false)) child = parent child === ancestor && break parent = child.parent::InferenceState diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl index 03f60062a9ebe..53c558cc95106 100644 --- a/test/compiler/effects.jl +++ b/test/compiler/effects.jl @@ -805,5 +805,16 @@ unknown_sparam_nothrow2(x::Ref{Ref{T}}) where T = (T; nothing) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow1, (Ref,))) @test Core.Compiler.is_nothrow(Base.infer_effects(unknown_sparam_nothrow2, (Ref{Ref{T}} where T,))) +# purely abstract recursion should not taint :terminates +# https://github.com/JuliaLang/julia/issues/48983 +abstractly_recursive1() = abstractly_recursive2() +abstractly_recursive2() = (Core.Compiler._return_type(abstractly_recursive1, Tuple{}); 1) +abstractly_recursive3() = abstractly_recursive2() +@test Core.Compiler.is_terminates(Base.infer_effects(abstractly_recursive3, ())) +actually_recursive1(x) = actually_recursive2(x) +actually_recursive2(x) = (x <= 0) ? 1 : actually_recursive1(x - 1) +actually_recursive3(x) = actually_recursive2(x) +@test !Core.Compiler.is_terminates(Base.infer_effects(actually_recursive3, (Int,))) + # https://github.com/JuliaLang/julia/issues/48856 @test Base.ismutationfree(Vector{Any}) == false