Skip to content

Commit

Permalink
effects: refine :nothrow when exct is known to be Bottom (#52270)
Browse files Browse the repository at this point in the history
Implements effects refinement that goes in the reverse direction of what
was implemented in 8dd0cf5.

In the discussion at #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
```
  • Loading branch information
aviatesk authored Nov 23, 2023
1 parent a624d44 commit 0c46852
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 8 deletions.
5 changes: 5 additions & 0 deletions base/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
8 changes: 6 additions & 2 deletions base/expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
Expand Down
12 changes: 12 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
4 changes: 1 addition & 3 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 0c46852

Please sign in to comment.