report throw calls appropriately

README.md

@@ -156,7 +156,7 @@ No errors !
 - more accurate error reports (especially builtin-calls, i.e. setup our own `tfunc`s)
 - report some cases of `throw` _appropriately_
   * e.g. we want to get reports for `rand(::Char) -> throw(ArgumentError("Sampler for this object is not defined"))`, while not for `sin(::Float64) -> throw(DomainError(x, "sin(x) is only defined for finite x."))`
-  * to address above case, we need some "inter-frame" analysis
+  * the above case is already half-solved by a simple inter-frame analysis with there's possibility of false negatives
 - report performance pitfalls
 - incremental profiling (for fast watch mode)
 - support virtual package loading (without actual loading, circumventing Revise.jl's limitation)

src/TypeProfiler.jl

@@ -12,7 +12,7 @@ import Core.Compiler:
     # abstractinterpretation.jl
     abstract_call_gf_by_type, abstract_call_known, abstract_call,
     abstract_eval_special_value, abstract_eval_value_expr, abstract_eval_value,
-    abstract_eval_statement, builtin_tfunction, typeinf_local, typeinf_edge,
+    abstract_eval_statement, builtin_tfunction, typeinf_local, typeinf_edge, finish,
     # tpcache.jl
     WorldView
 
@@ -28,7 +28,7 @@ import Core.Compiler:
     InternalCodeCache, CodeInstance, WorldRange,
     MethodInstance, Bottom, NOT_FOUND, MethodMatchInfo, UnionSplitInfo, MethodLookupResult,
     Const, VarTable, SSAValue, SlotNumber, Slot, slot_id, GlobalRef, GotoIfNot, ReturnNode,
-    widenconst, isconstType, typeintersect, ⊑, Builtin, CallMeta,
+    widenconst, isconstType, typeintersect, ⊑, Builtin, CallMeta, is_throw_call,
     argtypes_to_type, abstract_eval_ssavalue, _methods_by_ftype, specialize_method, typeinf
 
 import Base:

src/abstractinterpretation.jl

@@ -158,5 +158,39 @@ end
 The setter and getter of a frame that `interp` is currently profiling.
 Current frame is needed when we assemble virtual stack frame from cached error reports.
 """
-set_current_frame!(interp::TPInterpreter, frame::InferenceState) = interp.frame[] = frame
-get_current_frame(interp::TPInterpreter) = interp.frame[]
+set_current_frame!(interp::TPInterpreter, frame::InferenceState) = interp.current_frame[] = frame
+get_current_frame(interp::TPInterpreter) = interp.current_frame[]
+
+# here we can work on `InferenceState` that inference already ran on it,
+# and also maybe optimization has been done
+function finish(me::InferenceState, interp::TPInterpreter)
+    ret = invoke(finish, Tuple{InferenceState,AbstractInterpreter}, me, interp)
+
+    # report `throw` calls "appropriately" by simple inter-frame analysis
+    # the basic stance here is really conservative so we don't report them unless they
+    # will be inevitably called can won't be caught by `try/catch` in frame at any level
+    # NOTE:
+    # this is better to happen here (after the optimization) to reduce the chance of false
+    # negative reports
+    if get_result(me) === Bottom
+        # report `throw`s only if there is no circumvent pass, which is represented by
+        # `Bottom`-annotated return type inference with non-empty `throw` blocks
+        throw_calls = filter(is_throw_call′, me.src.code)
+        if !isempty(throw_calls)
+            push!(interp.exceptionreports, length(interp.reports) => ExceptionReport(me, interp, throw_calls))
+        end
+
+        if isroot(me)
+            # if return type is `Bottom`-annotated for root frame, this means some error(s)
+            # get propagated here, let's report `ExceptionReport` if exist
+            for (i, (idx, report)) in enumerate(interp.exceptionreports)
+                insert!(interp.reports, idx + i, report)
+            end
+        end
+    end
+
+    return ret
+end
+
+is_throw_call′(@nospecialize(_)) = false
+is_throw_call′(e::Expr)          = is_throw_call(e)

src/abstractinterpreterinterface.jl

@@ -8,8 +8,13 @@ struct TPInterpreter <: AbstractInterpreter
     discard_trees::Bool
 
     # TypeProfiler.jl specific
-    id::Symbol                 # for escaping reporting duplicated cached reports
-    frame::Ref{InferenceState} # for constructing virtual stack frame from cached reports
+    # for escaping reporting duplicated cached reports
+    id::Symbol
+    # for constructing virtual stack frame from cached reports
+    current_frame::Ref{InferenceState}
+    # keeping reports from frame that always `throw`
+    exceptionreports::Vector{Pair{Int,ExceptionReport}}
+
     istoplevel::Bool
     virtualglobalvartable::Dict{Module,Dict{Symbol,Any}} # maybe we don't need this nested dicts
     filter_native_remarks::Bool
@@ -37,6 +42,7 @@ struct TPInterpreter <: AbstractInterpreter
                    discard_trees,
                    id,
                    Ref{InferenceState}(),
+                   [],
                    istoplevel,
                    virtualglobalvartable,
                    filter_native_remarks,

src/reports.jl

@@ -136,6 +136,47 @@ end
 
 @reportdef NonBooleanCondErrorReport(interp, sv, @nospecialize(t))
 
+"""
+    ExceptionReport <: InferenceErrorReport
+
+Represents general `Exception`s traced during inference. They are reported only when there's
+  "inevitable" [`throw`](@ref) calls found by inter-frame analysis.
+"""
+struct ExceptionReport <: InferenceErrorReport
+    st::VirtualStackTrace
+    msg::String
+    sig::Vector{Any}
+
+    # the constructors below are needed to be special cased, since `ExceptionReport` is
+    # reported _after_ the inference on `sv::InferenceState` has been done rather than
+    # during abstract interpretation
+
+    # inner constructor (from abstract interpretation)
+    function ExceptionReport(sv::InferenceState, interp, throw_calls)
+        msg = get_msg(ExceptionReport, interp, sv, throw_calls)
+        sig = get_sig(ExceptionReport, interp, sv, throw_calls)
+
+        cache_report! = generate_report_cacher(ExceptionReport, msg, sig, interp)
+
+        st = let
+            local sig = Any["unreachable"]
+            file, line = get_file_line(sv.linfo) # just use this frame's location
+            frame = (; file, line, sig)
+            st = VirtualFrame[frame]
+            cache_report!(sv, st)
+
+            isroot(sv) ? st : track_abstract_call_stack!(cache_report!, sv.parent, st) # postwalk
+        end
+
+        return new(reverse(st), msg, sig)
+    end
+
+    # inner constructor (from cache)
+    function ExceptionReport(st::VirtualStackTrace, msg::AbstractString, sig::AbstractVector)
+        return new(reverse(st), msg, sig)
+    end
+end
+
 """
     NativeRemark <: InferenceErrorReport
 
@@ -197,6 +238,18 @@ get_file_line(linfo::MethodInstance) = linfo.def.file, linfo.def.line
 get_sig(::Type{<:InferenceErrorReport}, interp, sv, @nospecialize(args...)) = get_sig(sv)
 get_sig(sv::InferenceState) = _get_sig(sv, get_cur_stmt(sv))
 
+# special case `ExceptionReport`: there might be multiple throw calls in frame
+function get_sig(::Type{ExceptionReport}, interp, sv, throw_calls)
+    sig = Any[]
+    ncalls = length(throw_calls)
+    for (i, call) in enumerate(throw_calls)
+        call_sig = _get_sig(sv, call)
+        append!(sig, call_sig)
+        i ≠ ncalls && push!(sig, ", ")
+    end
+    return sig
+end
+
 _get_sig(args...) = first(_get_sig_type(args...))
 
 function _get_sig_type(sv::InferenceState, expr::Expr)
@@ -264,6 +317,14 @@ get_msg(::Type{UndefVarErrorReport}, interp, sv, mod, name) = isnothing(mod) ?
     "variable $(mod).$(name) is not defined"
 get_msg(::Type{NonBooleanCondErrorReport}, interp, sv, @nospecialize(t)) =
     "non-boolean ($(t)) used in boolean context"
+function get_msg(::Type{ExceptionReport}, interp, sv, throw_blocks)
+    n = length(throw_blocks)
+    return if isone(n)
+        "will throw"
+    else
+        "will throw either of"
+    end
+end
 get_msg(::Type{NativeRemark}, interp, sv, s) = s
 
 # utils

src/tpcache.jl

@@ -11,10 +11,20 @@ const TPCACHE = Dict{UInt,Pair{Symbol,Vector{InferenceReportCache}}}()
 
 get_id(interp::TPInterpreter) = interp.id
 
-function get_report_cache(sv, cache::InferenceReportCache{T}) where {T<:InferenceErrorReport}
+function restore_cached_report!(cache::InferenceReportCache{T}, interp) where {T<:InferenceErrorReport}
+    sv = get_current_frame(interp)
     cur_st = track_abstract_call_stack!((args...)->nothing, sv)
     st = vcat(cache.st, cur_st)
-    return T(st, cache.msg, cache.sig)
+    report = T(st, cache.msg, cache.sig)
+    push!(interp.reports, report)
+end
+
+function restore_cached_report!(cache::InferenceReportCache{ExceptionReport}, interp)
+    sv = get_current_frame(interp)
+    cur_st = track_abstract_call_stack!((args...)->nothing, sv)
+    st = vcat(cache.st, cur_st)
+    report = ExceptionReport(st, cache.msg, cache.sig)
+    push!(interp.exceptionreports, length(interp.reports) => report)
 end
 
 # code cache interface
@@ -44,10 +54,8 @@ function CC.get(tpc::TPCache, mi::MethodInstance, default)
 
             # don't append duplicated reports from the same inference process
             if id !== get_id(tpc.interp)
-                frame = get_current_frame(tpc.interp)
                 for cache in cached_reports
-                    report = get_report_cache(frame, cache)
-                    push!(tpc.interp.reports, report)
+                    restore_cached_report!(cache, tpc.interp)
                 end
             end
         end

test/runtests.jl

@@ -1,10 +1,13 @@
+# %% setup
+# --------
+
 using Test, TypeProfiler, InteractiveUtils
 
 import Core.Compiler:
     widenconst
 
 import TypeProfiler:
-    TPInterpreter, generate_virtual_lambda, profile_call, profile_call_gf!,
+    TPInterpreter, generate_virtual_lambda, profile_call, profile_call_gf, profile_call_gf!,
     virtual_process!, report_errors, getvirtualglobalvar,
     ToplevelErrorReport, InferenceErrorReport
 
@@ -43,19 +46,18 @@ function profile_toplevel!(s,
     return virtual_process!(s, filename, actualmodsym, virtualmod, interp)
 end
 
-macro gen_lambda(ex)
-    return :($(generate_virtual_lambda)($(__module__), $(__source__), $(QuoteNode(ex))))
-end
+# %% test body
+# ------------
 
-@testset "virtualprocess.jl" begin
+@testset "virtual process" begin
     include("test_virtualprocess.jl")
 end
 
-@testset "abstractinterpretation.jl" begin
+@testset "abstract interpretation" begin
     include("test_abstractinterpretation.jl")
 end
 
-@testset "tfuncs.jl" begin
+@testset "tfuncs" begin
     include("test_tfuncs.jl")
 end
 

test/test_abstractinterpretation.jl

@@ -12,3 +12,49 @@
         test_sum_over_string(res)
     end
 end
+
+@testset "report `throw` calls" begin
+    # simplest case
+    let
+        interp, frame = profile_call(()->throw("foo"))
+        @test !isempty(interp.reports)
+        @test first(interp.reports) isa ExceptionReport
+    end
+
+    # throws in deep level
+    let
+        foo(a) = throw(a)
+        interp, frame = profile_call((a)->foo(a), 1)
+        @test !isempty(interp.reports)
+        @test first(interp.reports) isa ExceptionReport
+    end
+
+    # don't report possibly false negative `throw`s
+    let
+        foo(a) = a ≤ 0 ? throw("a is $(a)") : a
+        λ = a -> foo(a)
+        interp, frame = profile_call_gf(Tuple{typeof(λ),Int})
+        @test isempty(interp.reports)
+    end
+
+    # constant propagation sometimes helps exclude false negatives
+    let
+        foo(a) = a ≤ 0 ? throw("a is $(a)") : a
+        λ = () -> foo(0)
+        interp, frame = profile_call_gf(Tuple{typeof(λ)})
+        @test !isempty(interp.reports)
+        @test first(interp.reports) isa ExceptionReport
+    end
+
+    # end to end
+    let
+        # this should report `throw(ArgumentError("Sampler for this object is not defined")`
+        interp, frame = profile_call(rand, '1')
+        @test !isempty(interp.reports)
+        @test first(interp.reports) isa ExceptionReport
+
+        # this should not report `throw(DomainError(x, "sin(x) is only defined for finite x."))`
+        interp, frame = profile_call(sin, 1)
+        @test isempty(interp.reports)
+    end
+end