Skip to content

Commit

Permalink
wip: JETAnalyzer: avoid duplicated reports
Browse files Browse the repository at this point in the history
  • Loading branch information
@aviatesk
aviatesk committed Apr 18, 2023
1 parent c0adbac commit bbbeec3
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 56 deletions.
2 changes: 2 additions & 0 deletions src/abstractinterpret/abstractanalyzer.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ accessed using `get_reports(analyzer, result)`.
"""
struct AnalysisResult
reports::Vector{InferenceErrorReport}
reported::BitSet
end
AnalysisResult(reports::Vector{InferenceErrorReport}) = AnalysisResult(reports, BitSet())

"""
CachedAnalysisResult
Expand Down
80 changes: 45 additions & 35 deletions src/analyzers/jetanalyzer.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ JETInterface.AnalysisCache(analyzer::JETAnalyzer) = analyzer.analysis_cache

const JET_ANALYZER_CACHE = IdDict{UInt, AnalysisCache}()

# avoid duplicated reports
function add_new_report!(analyzer::JETAnalyzer, sv::InferenceState, @nospecialize(report::InferenceErrorReport))
result = analyzer[sv.result]
sv.currpc in result.reported && return nothing
push!(result.reported, sv.currpc)
report = add_new_report!(analyzer, sv.result, report)
return report
end

# report passes
# =============

Expand Down Expand Up @@ -663,12 +672,12 @@ end
function report_method_error!(analyzer::JETAnalyzer,
sv::InferenceState, info::MethodMatchInfo, @nospecialize(atype), @nospecialize(rt), sound::Bool)
if is_empty_match(info)
add_new_report!(analyzer, sv.result, MethodErrorReport(sv, atype, 0, #=uncovered=#false))
add_new_report!(analyzer, sv, MethodErrorReport(sv, atype, 0, #=uncovered=#false))
return true
elseif sound && !is_fully_covered(info)
report = MethodErrorReport(sv, atype, 0, #=uncovered=#true)
report.sig[end] = widenconst(ignorelimited(rt))
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
return false
Expand Down Expand Up @@ -710,12 +719,12 @@ function report_method_error_for_union_split!(analyzer::JETAnalyzer,
end
reported = false
if empty_matches !== nothing
add_new_report!(analyzer, sv.result, MethodErrorReport(sv, empty_matches..., #=reason=#false))
add_new_report!(analyzer, sv, MethodErrorReport(sv, empty_matches..., #=reason=#false))
reported |= true
end
if uncovered_matches !== nothing
report = MethodErrorReport(sv, uncovered_matches..., #=uncovered=#true)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
report.sig[end] = widenconst(ignorelimited(rt))
reported |= true
end
Expand All @@ -740,7 +749,7 @@ function (::SoundPass)(::Type{UnanalyzedCallReport}, analyzer::JETAnalyzer,
if call.info === false
@assert call.rt === Any "unexpected call info"
report = UnanalyzedCallReport(sv, atype)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
report.sig[end] = Any
return true
end
Expand All @@ -759,7 +768,7 @@ function (::SoundBasicPass)(::Type{InvalidReturnTypeCall}, analyzer::AbstractAna
# `Core.Compiler.NativeInterpreter` doesn't also (it hard-codes the return type as `Type`)
if length(argtypes) ≠ 3
# invalid argument #, let's report and return error result (i.e. `Bottom`)
add_new_report!(analyzer, sv.result, InvalidReturnTypeCall(sv))
add_new_report!(analyzer, sv, InvalidReturnTypeCall(sv))
return true
end
return false
Expand Down Expand Up @@ -802,7 +811,7 @@ function (::SoundBasicPass)(::Type{InvalidInvokeErrorReport}, analyzer::JETAnaly
# if the error type (`Bottom`) is propagated from the `invoke`d call, the error has
# already been reported within `typeinf_edge`, so ignore that case
if !isa(ret.info, InvokeCallInfo)
add_new_report!(analyzer, sv.result, InvalidInvokeErrorReport(sv, argtypes))
add_new_report!(analyzer, sv, InvalidInvokeErrorReport(sv, argtypes))
return true
end
end
Expand Down Expand Up @@ -845,7 +854,7 @@ function report_undef_global_var!(analyzer::JETAnalyzer, sv::InferenceState, gr:
ccall(:jl_binding_type, Any, (Any, Any), gr.mod, gr.name) !== nothing && return false
end
begin @label report
add_new_report!(analyzer, sv.result, UndefVarErrorReport(sv, gr))
add_new_report!(analyzer, sv, UndefVarErrorReport(sv, gr))
return true
end
end
Expand Down Expand Up @@ -876,11 +885,11 @@ function report_undef_static_parameter!(analyzer::JETAnalyzer, sv::InferenceStat
if 1 ≤ n ≤ length(sv.sptypes)
if (@static VERSION ≥ v"1.10.0-DEV.556" ? sv.sptypes[n].typ : sv.sptypes[n]) === Any
tv = sv.linfo.sparam_vals[n]
add_new_report!(analyzer, sv.result, UndefVarErrorReport(sv, tv))
add_new_report!(analyzer, sv, UndefVarErrorReport(sv, tv))
return true
end
else
add_new_report!(analyzer, sv.result, UndefVarErrorReport(sv, TypeVar(:unknown)))
add_new_report!(analyzer, sv, UndefVarErrorReport(sv, TypeVar(:unknown)))
return true
end
return false
Expand All @@ -899,15 +908,15 @@ function (::SoundPass)(::Type{UndefVarErrorReport}, analyzer::JETAnalyzer, sv::I
var::SlotNumber, vtypes::VarTable, @nospecialize(ret))
vtyp = vtypes[slot_id(var)]
if vtyp.undef
add_new_report!(analyzer, sv.result, UndefVarErrorReport(sv, get_slotname(sv, var)))
add_new_report!(analyzer, sv, UndefVarErrorReport(sv, get_slotname(sv, var)))
return true
end
return false
end
function (::BasicPass)(::Type{UndefVarErrorReport}, analyzer::JETAnalyzer, sv::InferenceState,
var::SlotNumber, vtypes::VarTable, @nospecialize(ret))
ret === Bottom || return false
add_new_report!(analyzer, sv.result, UndefVarErrorReport(sv, get_slotname(sv, var)))
add_new_report!(analyzer, sv, UndefVarErrorReport(sv, get_slotname(sv, var)))
return true
end

Expand Down Expand Up @@ -944,7 +953,7 @@ function report_global_assignment!(analyzer::JETAnalyzer,
if btyp !== nothing
vtyp = widenconst(vtyp)
if !(sound ? vtyp ⊑ btyp : hasintersect(vtyp, btyp))
add_new_report!(analyzer, sv.result, InvalidGlobalAssignmentError(sv, vtyp, btyp, mod, name))
add_new_report!(analyzer, sv, InvalidGlobalAssignmentError(sv, vtyp, btyp, mod, name))
return true
end
return false
Expand Down Expand Up @@ -1006,27 +1015,18 @@ function report_non_boolean_cond!(analyzer::JETAnalyzer, sv::InferenceState, @no
end
end
if info !== nothing
add_new_report!(analyzer, sv.result, NonBooleanCondErrorReport(sv, info..., #=uncovered=#check_uncovered))
add_new_report!(analyzer, sv, NonBooleanCondErrorReport(sv, info..., #=uncovered=#check_uncovered))
return true
end
else
if !(check_uncovered ? t ⊑ Bool : hasintersect(t, Bool))
add_new_report!(analyzer, sv.result, NonBooleanCondErrorReport(sv, t, 0, #=uncovered=#check_uncovered))
add_new_report!(analyzer, sv, NonBooleanCondErrorReport(sv, t, 0, #=uncovered=#check_uncovered))
return true
end
end
return false
end

function (::SoundBasicPass)(::Type{InvalidConstantRedefinition}, analyzer::JETAnalyzer, sv::InferenceState, mod::Module, name::Symbol, @nospecialize(prev_t), @nospecialize(t))
add_new_report!(analyzer, sv.result, InvalidConstantRedefinition(sv, mod, name, prev_t, t))
return true
end
function (::SoundBasicPass)(::Type{InvalidConstantDeclaration}, analyzer::JETAnalyzer, sv::InferenceState, mod::Module, name::Symbol)
add_new_report!(analyzer, sv.result, InvalidConstantDeclaration(sv, mod, name))
return true
end

# XXX tfunc implementations in Core.Compiler are really not enough to catch invalid calls
# TODO set up our own checks and enable sound analysis

Expand Down Expand Up @@ -1059,12 +1059,12 @@ function report_serious_exception!(analyzer::JETAnalyzer, sv::InferenceState, ar
if isa(a, Const)
err = a.val
if isa(err, UndefKeywordError)
add_new_report!(analyzer, sv.result, SeriousExceptionReport(sv, err, get_lin((sv, get_currpc(sv)))))
add_new_report!(analyzer, sv, SeriousExceptionReport(sv, err, get_lin((sv, get_currpc(sv)))))
return true
elseif isa(err, MethodError)
# ignore https://github.com/JuliaLang/julia/blob/7409a1c007b7773544223f0e0a2d8aaee4a45172/base/boot.jl#L261
if err.f !== Bottom
add_new_report!(analyzer, sv.result, SeriousExceptionReport(sv, err, get_lin((sv, get_currpc(sv)))))
add_new_report!(analyzer, sv, SeriousExceptionReport(sv, err, get_lin((sv, get_currpc(sv)))))
return true
end
end
Expand Down Expand Up @@ -1200,7 +1200,7 @@ function (::BasicPass)(::Type{AbstractBuiltinErrorReport}, analyzer::JETAnalyzer
if @static VERSION >= v"1.10.0-DEV.197" ? (ret isa IntrinsicError) : false
msg = LazyString(f, ": ", ret.reason)
report = BuiltinErrorReport(sv, f, argtypes, msg, #=print_signature=#true)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
return handle_invalid_builtins!(analyzer, sv, f, argtypes, ret)
Expand Down Expand Up @@ -1311,7 +1311,7 @@ function report_fieldaccess!(analyzer::JETAnalyzer, sv::InferenceState, @nospeci
if !_mutability_errorcheck(s00)
msg = lazy"setfield!: immutable struct of type $s00 cannot be changed"
report = BuiltinErrorReport(sv, setfield!, argtypes, msg)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
end
Expand All @@ -1333,14 +1333,14 @@ function report_fieldaccess!(analyzer::JETAnalyzer, sv::InferenceState, @nospeci
if s <: Module
if issetfield!
report = BuiltinErrorReport(sv, setfield!, argtypes, MODULE_SETFIELD_MSG)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
nametyp = widenconst(name)
if !hasintersect(nametyp, Symbol)
msg = type_error_msg(getglobal, Symbol, nametyp)
report = BuiltinErrorReport(sv, getglobal, argtypes, msg)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
end
Expand All @@ -1361,7 +1361,7 @@ function report_fieldaccess!(analyzer::JETAnalyzer, sv::InferenceState, @nospeci
else
@assert false "invalid field analysis"
end
add_new_report!(analyzer, sv.result, BuiltinErrorReport(sv, f, argtypes, msg))
add_new_report!(analyzer, sv, BuiltinErrorReport(sv, f, argtypes, msg))
return true
end

Expand All @@ -1383,7 +1383,7 @@ function report_divide_error!(analyzer::JETAnalyzer, sv::InferenceState, @nospec
if isprimitivetype(t) && t <: Number
if isa(a, Const) && a.val === zero(t)
report = BuiltinErrorReport(sv, f, argtypes, DIVIDE_ERROR_MSG)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
end
Expand All @@ -1395,7 +1395,7 @@ function handle_invalid_builtins!(analyzer::JETAnalyzer, sv::InferenceState, @no
if ret === Bottom
msg = GENERAL_BUILTIN_ERROR_MSG
report = BuiltinErrorReport(sv, f, argtypes, msg, #=print_signature=#true)
add_new_report!(analyzer, sv.result, report)
add_new_report!(analyzer, sv, report)
return true
end
return false
Expand All @@ -1416,18 +1416,28 @@ function (::SoundPass)(::Type{AbstractBuiltinErrorReport}, analyzer::JETAnalyzer
@assert !(f === throw) "`throw` calls should be handled either by the report pass of `SeriousExceptionReport` or `UncaughtExceptionReport`"
if isa(f, IntrinsicFunction)
if !Core.Compiler.intrinsic_nothrow(f, argtypes)
add_new_report!(analyzer, sv.result, UnsoundBuiltinErrorReport(sv, f, argtypes))
add_new_report!(analyzer, sv, UnsoundBuiltinErrorReport(sv, f, argtypes))
end
else
nothrow = !(@static isdefined(CC, :typeinf_lattice) ?
Core.Compiler.builtin_nothrow(CC.typeinf_lattice(analyzer), f, argtypes, rt) :
Core.Compiler.builtin_nothrow(f, argtypes, rt))
if nothrow
add_new_report!(analyzer, sv.result, UnsoundBuiltinErrorReport(sv, f, argtypes))
add_new_report!(analyzer, sv, UnsoundBuiltinErrorReport(sv, f, argtypes))
end
end
end

# top-level
function (::SoundBasicPass)(::Type{InvalidConstantRedefinition}, analyzer::JETAnalyzer, sv::InferenceState, mod::Module, name::Symbol, @nospecialize(prev_t), @nospecialize(t))
add_new_report!(analyzer, sv.result, InvalidConstantRedefinition(sv, mod, name, prev_t, t))
return true
end
function (::SoundBasicPass)(::Type{InvalidConstantDeclaration}, analyzer::JETAnalyzer, sv::InferenceState, mod::Module, name::Symbol)
add_new_report!(analyzer, sv.result, InvalidConstantDeclaration(sv, mod, name))
return true
end

# entries
# =======

Expand Down
42 changes: 21 additions & 21 deletions test/analyzers/test_jetanalyzer.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,27 +148,27 @@ end
@test report.sig[end] === Symbol
end

# report both no match error and uncovered match error
let result = @eval Module() begin
onlyint(::Int) = :ok
$report_call((Union{Integer,Nothing},); mode=:sound) do x
onlyint(x)
end
end
@test length(get_reports_with_test(result)) == 2
@test any(get_reports_with_test(result)) do report
# no match for `onlyint(::Nothing)`
report isa MethodErrorReport &&
!report.uncovered &&
report.sig[end] === Union{}
end
@test any(get_reports_with_test(result)) do report
# uncovered match for `onlyint(::Integer)`
report isa MethodErrorReport &&
report.uncovered &&
report.sig[end] === Symbol
end
end
# # report both no match error and uncovered match error
# let result = @eval Module() begin
# onlyint(::Int) = :ok
# $report_call((Union{Integer,Nothing},); mode=:sound) do x
# onlyint(x)
# end
# end
# @test length(get_reports_with_test(result)) == 2
# @test any(get_reports_with_test(result)) do report
# # no match for `onlyint(::Nothing)`
# report isa MethodErrorReport &&
# !report.uncovered &&
# report.sig[end] === Union{}
# end
# @test any(get_reports_with_test(result)) do report
# # uncovered match for `onlyint(::Integer)`
# report isa MethodErrorReport &&
# report.uncovered &&
# report.sig[end] === Symbol
# end
# end
end

@testset "UndefVarErrorReport" begin
Expand Down

0 comments on commit bbbeec3

Please sign in to comment.