diff --git a/src/abstractinterpretation.jl b/src/abstractinterpretation.jl index 37825dd54..ee3ac7145 100644 --- a/src/abstractinterpretation.jl +++ b/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,7 +99,8 @@ 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 @@ -103,14 +108,13 @@ function abstract_call_gf_by_type(interp::TPInterpreter, @nospecialize(f), argty 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 diff --git a/src/abstractinterpreterinterface.jl b/src/abstractinterpreterinterface.jl index 58a785c7c..9b49d2a0b 100644 --- a/src/abstractinterpreterinterface.jl +++ b/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 diff --git a/src/reports.jl b/src/reports.jl index 901de6ec7..d5361ed0e 100644 --- a/src/reports.jl +++ b/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) = diff --git a/src/tfuncs.jl b/src/tfuncs.jl index f8191a387..88a5f2035 100644 --- a/src/tfuncs.jl +++ b/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` diff --git a/test/runtests.jl b/test/runtests.jl index 712ec0716..1e2ae23f0 100644 --- a/test/runtests.jl +++ b/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