From 0c46852901c63b33f0603b0afd58ff1da687d760 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Thu, 23 Nov 2023 17:00:46 +0900 Subject: [PATCH] effects: refine `:nothrow` when `exct` is known to be `Bottom` (#52270) Implements effects refinement that goes in the reverse direction of what was implemented in 8dd0cf5. In the discussion at JuliaLang/julia#52268, there's a debate on how to deal with exceptions like `StackOverflowError` or `InterruptException` that are caused by environment. The original purpose of #52268 was to fix issues where methods that could cause a `StackOverflowError` were mistakenly refined to `:nothrow`. But now it feels like it's a bit weird to model such environment-dependent exceptions as `:nothrow` in the first place. If we can exclude environment-dependent errors from the `:nothrow` modeling, those methods might be safely considered `:nothrow`. To prevent the concrete evaluation of methods that may cause `StackOverflowError`, it's enough to taint its `:teminates` after all. This commit excludes environment-depending errors from the `:nothrow` modeling, reverting the changes from #52268 while making it explicit in the `Effects` and `Base.@assume_effects` docstrings. Anyway, now the compile is able to prove `:nothrow`-ness of the following frame: ```julia function getindex_nothrow(xs::Vector{Int}, i::Int) try return xs[i] catch err err isa BoundsError && return nothing rethrow(err) end end ``` --- base/compiler/effects.jl | 5 +++++ base/compiler/typeinfer.jl | 8 +++++--- base/expr.jl | 8 ++++++-- test/compiler/effects.jl | 12 ++++++++++++ test/compiler/inference.jl | 4 +--- 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/base/compiler/effects.jl b/base/compiler/effects.jl index d4c972ab89e95..9d66407b58eb3 100644 --- a/base/compiler/effects.jl +++ b/base/compiler/effects.jl @@ -24,6 +24,11 @@ following meanings: * `EFFECT_FREE_IF_INACCESSIBLEMEMONLY`: the `:effect-free`-ness of this method can later be refined to `ALWAYS_TRUE` in a case when `:inaccessiblememonly` is proven. - `nothrow::Bool`: this method is guaranteed to not throw an exception. + If the execution of this method may raise `MethodError`s and similar exceptions, then + the method is not considered as `:nothrow`. + However, note that environment-dependent errors like `StackOverflowError` or `InterruptException` + are not modeled by this effect and thus a method that may result in `StackOverflowError` + does not necessarily need to taint `:nothrow` (although it should usually taint `:terminates` too). - `terminates::Bool`: this method is guaranteed to terminate. - `notaskstate::Bool`: this method does not access any state bound to the current task and may thus be moved to a different task without changing observable diff --git a/base/compiler/typeinfer.jl b/base/compiler/typeinfer.jl index 4075f25c6b1e0..958993847a48e 100644 --- a/base/compiler/typeinfer.jl +++ b/base/compiler/typeinfer.jl @@ -463,6 +463,11 @@ function adjust_effects(sv::InferenceState) # always throwing an error counts or never returning both count as consistent ipo_effects = Effects(ipo_effects; consistent=ALWAYS_TRUE) end + if sv.exc_bestguess === Bottom + # if the exception type of this frame is known to be `Bottom`, + # this frame is known to be safe + ipo_effects = Effects(ipo_effects; nothrow=true) + end if is_inaccessiblemem_or_argmemonly(ipo_effects) && all(1:narguments(sv, #=include_va=#true)) do i::Int return is_mutation_free_argtype(sv.slottypes[i]) end @@ -883,9 +888,6 @@ function typeinf_edge(interp::AbstractInterpreter, method::Method, @nospecialize update_valid_age!(caller, frame.valid_worlds) effects = adjust_effects(Effects(), method) exc_bestguess = refine_exception_type(frame.exc_bestguess, effects) - # this call can fail into an infinite cycle, so incorporate this fact into - # `exc_bestguess` by merging `StackOverflowError` into it - exc_bestguess = tmerge(typeinf_lattice(interp), Core.StackOverflowError, exc_bestguess) return EdgeCallResult(frame.bestguess, exc_bestguess, nothing, effects) end diff --git a/base/expr.jl b/base/expr.jl index 3e68c68b2ac4f..755eff1b7c1b8 100644 --- a/base/expr.jl +++ b/base/expr.jl @@ -552,7 +552,7 @@ were not executed. --- ## `:nothrow` -The `:nothrow` settings asserts that this method does not terminate abnormally +The `:nothrow` settings asserts that this method does not throw an exception (i.e. will either always return a value or never return). !!! note @@ -561,7 +561,11 @@ The `:nothrow` settings asserts that this method does not terminate abnormally method itself. !!! note - `MethodErrors` and similar exceptions count as abnormal termination. + If the execution of a method may raise `MethodError`s and similar exceptions, then + the method is not considered as `:nothrow`. + However, note that environment-dependent errors like `StackOverflowError` or `InterruptException` + are not modeled by this effect and thus a method that may result in `StackOverflowError` + does not necessarily need to be `!:nothrow` (although it should usually be `!:terminates` too). --- ## `:terminates_globally` diff --git a/test/compiler/effects.jl b/test/compiler/effects.jl index 133ff6e9f3ca9..3e0c24f908bc9 100644 --- a/test/compiler/effects.jl +++ b/test/compiler/effects.jl @@ -1290,3 +1290,15 @@ end |> !Core.Compiler.is_noub @test Base.infer_effects((Vector{Any},Int)) do xs, i @inbounds getindex_dont_propagate(xs, i) end |> Core.Compiler.is_noub + +# refine `:nothrow` when `exct` is known to be `Bottom` +@test Base.infer_exception_type(getindex, (Vector{Int},Int)) == BoundsError +function getindex_nothrow(xs::Vector{Int}, i::Int) + try + return xs[i] + catch err + err isa BoundsError && return nothing + rethrow(err) + end +end +@test Core.Compiler.is_nothrow(Base.infer_effects(getindex_nothrow, (Vector{Int}, Int))) diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index 108a62db4b940..9e06d120e501f 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -423,13 +423,11 @@ A14009(a::T) where {T} = A14009{T}() f14009(a) = rand(Bool) ? f14009(A14009(a)) : a code_typed(f14009, (Int,)) code_llvm(devnull, f14009, (Int,)) -@test Base.infer_exception_type(f14009, (Int,)) != Union{} mutable struct B14009{T}; end g14009(a) = g14009(B14009{a}) code_typed(g14009, (Type{Int},)) code_llvm(devnull, g14009, (Type{Int},)) -@test Base.infer_exception_type(g14009, (Type{Int},)) == StackOverflowError # issue #9232 arithtype9232(::Type{T},::Type{T}) where {T<:Real} = arithtype9232(T) @@ -5560,7 +5558,7 @@ function foo_typed_throw_metherr() end @test Base.return_types(foo_typed_throw_metherr) |> only === Float64 -# using `exct` information if `:nothrow` is proven +# refine `exct` when `:nothrow` is proven Base.@assume_effects :nothrow function sin_nothrow(x::Float64) x == Inf && return zero(x) return sin(x)