exclude more false positive cases with constant propagation

src/abstractinterpretation.jl

@@ -60,6 +60,9 @@ function abstract_call_gf_by_type(interp::TPInterpreter, @nospecialize(f), argty
                                   max_methods::Int = InferenceParams(interp).MAX_METHODS)
     ret = (@invoke abstract_call_gf_by_type(interp::AbstractInterpreter, f, argtypes::Vector{Any}, atype, sv::InferenceState,
                                             max_methods::Int))::CallMeta
+
+    info = ret.info
+    linfo = sv.linfo
     is_const = is_constant_propagated_result(sv)
 
     # throw away previously-reported `NoMethodErrorReport` if we re-infer this frame with
@@ -74,7 +77,7 @@ function abstract_call_gf_by_type(interp::TPInterpreter, @nospecialize(f), argty
     #   cached `MethodInstance` of `setproperty!` against structs with multiple type fields
     #   will report union-split `NoMethodErrorReport` (that can be excluded by constant propagation)
     #   TODO: this can be wrong in some cases since actual errors that would happen without
-    #   the current constants can be threw away as well; hopefully another future 
+    #   the current constants can be threw away as well; hopefully another future
     #   constant propagation "re-reveals" the threw away errors, but of course this doesn't
     #   necessarily hold true always
     #
@@ -83,7 +86,8 @@ function abstract_call_gf_by_type(interp::TPInterpreter, @nospecialize(f), argty
     if is_const
         inds = findall(interp.reports) do report
             return isa(report, NoMethodErrorReport) &&
-                ⊑(atype, report.atype) # use `atype` as key
+                linfo == report.linfo # use `sv.linfo` as key
+                # ⊑(atype, report.atype) # use `atype` as key
         end
         isempty(inds) || deleteat!(interp.reports, inds)
 
@@ -95,22 +99,22 @@ function abstract_call_gf_by_type(interp::TPInterpreter, @nospecialize(f), argty
                 # @assert id === get_id(interp)
                 cached_inds = findall(cached_reports) do cached_report
                     return isa(cached_report, InferenceReportCache{NoMethodErrorReport}) &&
-                        ⊑(atype, last(#= atype =# cached_report.args)::Type) # use `atype` as key
+                        linfo == last(#= linfo =# cached_report.args)::MethodInstance # use `sv.linfo` as key
+                        # ⊑(atype, last(#= atype =# cached_report.args)::Type) # use `atype` as key
                 end
                 deleteat!(cached_reports, cached_inds)
             end
         end
     end
 
     # report no method error
-    info = ret.info
     if isa(info, UnionSplitInfo)
         # if `info` is `UnionSplitInfo`, but there won't be a case where `info.matches` is empty
         for info in info.matches
             if is_empty_match(info)
                 # no method match for this union split
                 # ret.rt = Bottom # maybe we want to be more strict on error cases ?
-                er = (is_const ? NoMethodErrorReportConst : NoMethodErrorReport)(interp, sv, true, atype)
+                er = (is_const ? NoMethodErrorReportConst : NoMethodErrorReport)(interp, sv, true, atype, linfo)
                 add_remark!(interp, sv, er)
             end
         end
@@ -119,7 +123,7 @@ function abstract_call_gf_by_type(interp::TPInterpreter, @nospecialize(f), argty
         # initialization (i.e. `Bottom`)
         # typeassert(ret.rt, TypeofBottom)
         # add_remark!(interp, sv, NoMethodErrorReport(interp, sv, false, atype))
-        er = (is_const ? NoMethodErrorReportConst : NoMethodErrorReport)(interp, sv, false, atype)
+        er = (is_const ? NoMethodErrorReportConst : NoMethodErrorReport)(interp, sv, false, atype, linfo)
         add_remark!(interp, sv, er)
     end
 

src/abstractinterpreterinterface.jl

@@ -14,24 +14,27 @@ struct TPInterpreter <: AbstractInterpreter
     # for escaping reporting duplicated cached reports, sequential assignment of virtual global variable
     id::Symbol
 
+    # profiling
+    # ---------
+    # reports found so far
+    reports::Vector{InferenceErrorReport}
+    # keeps `throw` calls that are not caught within a single frame
+    exception_reports::Vector{Pair{Int,ExceptionReport}}
+
     # caching
     # -------
-    # for constructing virtual stack frame from cached reports
+    # we need to access currently inferred `frame` via `TPInterpreter` to construct virtual stack frame from cached reports
     current_frame::Ref{InferenceState}
-    # keeping reports from frame that always `throw`
-    exception_reports::Vector{Pair{Int,ExceptionReport}}
 
     # virtual toplevel execution
     # --------------------------
     # module -> (sym -> (id, type, λ sym, method instance))
     virtual_globalvar_table::Dict{Module,Dict{Symbol,Tuple{Symbol,Any,Symbol,MethodInstance}}}
 
-    # profiling
-    # ---------
-    # disabling caching of native remarks can speed up profiling time
+    # configurations
+    # --------------
+    # disables caching of native remarks can speed up profiling time
     filter_native_remarks::Bool
-    # reports found so far
-    reports::Vector{InferenceErrorReport}
 
     function TPInterpreter(world::UInt = get_world_counter();
                            inf_params::InferenceParams = gen_inf_params(),
@@ -51,11 +54,11 @@ struct TPInterpreter <: AbstractInterpreter
                    compress,
                    discard_trees,
                    id,
-                   Ref{InferenceState}(),
                    [],
+                   [],
+                   Ref{InferenceState}(),
                    virtual_globalvar_table,
                    filter_native_remarks,
-                   [],
                    )
     end
 end

src/reports.jl

@@ -167,9 +167,9 @@ macro reportdef(ex, kwargs...)
     end
 end
 
-@reportdef NoMethodErrorReport(interp, sv, unionsplit::Bool, @nospecialize(atype::Type))
+@reportdef NoMethodErrorReport(interp, sv, unionsplit::Bool, @nospecialize(atype::Type), linfo::MethodInstance)
 
-@reportdef NoMethodErrorReportConst(interp, sv, unionsplit::Bool, @nospecialize(atype::Type)) dont_cache = true
+@reportdef NoMethodErrorReportConst(interp, sv, unionsplit::Bool, @nospecialize(atype::Type), linfo::MethodInstance) dont_cache = true
 
 @reportdef InvalidBuiltinCallErrorReport(interp, sv)
 
@@ -339,7 +339,7 @@ function _get_sig_type(::InferenceState, qn::QuoteNode)
 end
 _get_sig_type(::InferenceState, @nospecialize(x)) = Any[repr(x; context = :compact => true)], nothing
 
-get_msg(::Type{<:Union{NoMethodErrorReport,NoMethodErrorReportConst}}, interp, sv, unionsplit, @nospecialize(atype)) = unionsplit ?
+get_msg(::Type{<:Union{NoMethodErrorReport,NoMethodErrorReportConst}}, interp, sv, unionsplit, @nospecialize(atype), linfo) = unionsplit ?
     "for one of the union split cases, no matching method found for signature: " :
     "no matching method found for call signature: "
 get_msg(::Type{InvalidBuiltinCallErrorReport}, interp, sv) =

src/tfuncs.jl

@@ -34,7 +34,13 @@ end
 function return_type_tfunc(interp::TPInterpreter, argtypes::Vector{Any}, sv::InferenceState)
     if length(argtypes) !== 3
         # invalid argument number, let's report and return error result (i.e. `Bottom`)
-        add_remark!(interp, sv, NoMethodErrorReport(interp, sv, false, argtypes_to_type(argtypes) #= this is not necessary to be computed correctly, though =#))
+        add_remark!(interp, sv, NoMethodErrorReport(interp,
+                                                    sv,
+                                                    false,
+                                                    # this is not necessary to be computed correctly, though
+                                                    argtypes_to_type(argtypes),
+                                                    sv.linfo
+                                                    ))
         return Bottom
     else
         # don't recursively pass on `TPInterpreter` via `@invoke_native`

test/runtests.jl

@@ -97,6 +97,36 @@ end
     @testset "tfuncs" begin
         include("test_tfuncs.jl")
     end
+
+    @testset "is it truly necessary ?" begin
+        let
+            # constant propagation can help to exclude false positive alerts;
+            # well, this is really bad code so I rather feel we may want to have a report
+            # for this case
+            res, interp = @profile_toplevel begin
+                function foo(n)
+                    if n < 10
+                        return n
+                    else
+                        return "over 10"
+                    end
+                end
+
+                function bar(n)
+                    if n < 10
+                        return foo(n) + 1
+                    else
+                        return foo(n) * "+1"
+                    end
+                end
+
+                bar(1)
+                bar(10)
+            end
+
+            @test isempty(res.inference_error_reports)
+        end
+    end
 end
 
 # # favorite