Skip to content

Commit

Permalink
fix #44732, propagate effects of a thrown concrete call correctly (#4…
Browse files Browse the repository at this point in the history
  • Loading branch information
aviatesk authored Mar 29, 2022
1 parent c919811 commit f86b4ef
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 23 deletions.
33 changes: 15 additions & 18 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end
tristate_merge!(sv, effects)
push!(const_results, const_result)
if const_result !== nothing
any_const_result = true
end
any_const_result |= const_result !== nothing
this_rt = tmerge(this_rt, rt)
if bail_out_call(interp, this_rt, sv)
break
Expand Down Expand Up @@ -187,9 +185,7 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
end
tristate_merge!(sv, effects)
push!(const_results, const_result)
if const_result !== nothing
any_const_result = true
end
any_const_result |= const_result !== nothing
end
@assert !(this_conditional isa Conditional) "invalid lattice element returned from inter-procedural context"
seen += 1
Expand Down Expand Up @@ -704,12 +700,12 @@ function pure_eval_call(interp::AbstractInterpreter,
end
function _pure_eval_call(@nospecialize(f), arginfo::ArgInfo)
args = collect_const_args(arginfo)
try
value = Core._apply_pure(f, args)
return Const(value)
value = try
Core._apply_pure(f, args)
catch
return nothing
end
return Const(value)
end

function concrete_eval_eligible(interp::AbstractInterpreter,
Expand Down Expand Up @@ -743,17 +739,18 @@ function concrete_eval_call(interp::AbstractInterpreter,
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState)
concrete_eval_eligible(interp, f, result, arginfo, sv) || return nothing
args = collect_const_args(arginfo)
try
value = Core._call_in_world_total(get_world_counter(interp), f, args...)
if is_inlineable_constant(value) || call_result_unused(sv)
# If the constant is not inlineable, still do the const-prop, since the
# code that led to the creation of the Const may be inlineable in the same
# circumstance and may be optimizable.
return ConstCallResults(Const(value), ConstResult(result.edge, value), EFFECTS_TOTAL)
end
world = get_world_counter(interp)
value = try
Core._call_in_world_total(world, f, args...)
catch
# The evaulation threw. By :consistent-cy, we're guaranteed this would have happened at runtime
return ConstCallResults(Union{}, ConstResult(result.edge), result.edge_effects)
return ConstCallResults(Union{}, ConstResult(result.edge, result.edge_effects), result.edge_effects)
end
if is_inlineable_constant(value) || call_result_unused(sv)
# If the constant is not inlineable, still do the const-prop, since the
# code that led to the creation of the Const may be inlineable in the same
# circumstance and may be optimizable.
return ConstCallResults(Const(value), ConstResult(result.edge, EFFECTS_TOTAL, value), EFFECTS_TOTAL)
end
return nothing
end
Expand Down
6 changes: 3 additions & 3 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1330,10 +1330,10 @@ end

function const_result_item(result::ConstResult, state::InliningState)
if !isdefined(result, :result) || !is_inlineable_constant(result.result)
return compileable_specialization(state.et, result.mi, EFFECTS_TOTAL)
else
return ConstantCase(quoted(result.result))
return compileable_specialization(state.et, result.mi, result.effects)
end
@assert result.effects === EFFECTS_TOTAL
return ConstantCase(quoted(result.result))
end

function handle_cases!(ir::IRCode, idx::Int, stmt::Expr, @nospecialize(atype),
Expand Down
5 changes: 3 additions & 2 deletions base/compiler/stmtinfo.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ end

struct ConstResult
mi::MethodInstance
effects::Effects
result
ConstResult(mi::MethodInstance) = new(mi)
ConstResult(mi::MethodInstance, @nospecialize val) = new(mi, val)
ConstResult(mi::MethodInstance, effects::Effects) = new(mi, effects)

This comment has been minimized.

Copy link
@ianatol

ianatol Apr 5, 2022

Member

Noticed this today, don't think this is valid? @aviatesk

This comment has been minimized.

Copy link
@ianatol

ianatol Apr 6, 2022

Member

But ConstResult(mi::MethodInstance, effects::Effects) = new(mi, effects, nothing) here causes a bootstrap error. I'm so confused...

This comment has been minimized.

Copy link
@aviatesk

aviatesk Apr 6, 2022

Author Member

This is valid. This constructor is for a case when the result field is uninitialized.

ConstResult(mi::MethodInstance, effects::Effects, @nospecialize val) = new(mi, effects, val)
end

"""
Expand Down
22 changes: 22 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,28 @@ Base.@assume_effects :consistent :effect_free :terminates_globally consteval(
consteval(getindex, ___CONST_DICT___, :a)
end

# https://github.com/JuliaLang/julia/issues/44732
struct Component44732
v
end
struct Container44732
x::Union{Nothing,Component44732}
end

# NOTE make sure to prevent inference bail out
validate44732(::Component44732) = nothing
validate44732(::Any) = error("don't erase this error!")

function issue44732(c::Container44732)
validate44732(c.x)
return nothing
end

let src = code_typed1(issue44732, (Container44732,))
@test any(isinvoke(:validate44732), src.code)
end
@test_throws ErrorException("don't erase this error!") issue44732(Container44732(nothing))

global x44200::Int = 0
function f44200()
global x = 0
Expand Down

0 comments on commit f86b4ef

Please sign in to comment.