Skip to content

Commit 27203bb

Browse files
authored
Make world-age increments explicit (JuliaLang#56509)
This PR introduces a new, toplevel-only, syntax form `:worldinc` that semantically represents the effect of raising the current task's world age to the latest world for the remainder of the current toplevel evaluation (that context being an entry to `eval` or a module expression). For detailed motivation on why this is desirable, see JuliaLang#55145, which I won't repeat here, but the gist is that we never really defined when world-age increments and worse are inconsistent about it. This is something we need to figure out now, because the bindings partition work will make world age even more observable via bindings. Having created a mechanism for world age increments, the big question is one of policy, i.e. when should these world age increments be inserted. Several reasonable options exist: 1. After world-age affecting syntax constructs (as proprosed in JuliaLang#55145) 2. Option 1 + some reasonable additional cases that people rely on 3. Before any top level `call` expression 4. Before any expression at toplevel whatsover As an example, case, consider `a == a` at toplevel. Depending on the semantics that could either be the same as in local scope, or each of the four world age dependent lookups (three binding lookups, one method lookup) could (potentially) occur in a different world age. The general tradeoff here is between the risk of exposing the user to confusing world age errors and our ability to optimize top-level code (in general, any `:worldinc` statement will require us to fully pessimize or recompile all following code). This PR basically implements option 2 with the following semantics: 1. The interpreter explicit raises the world age only at `:worldinc` exprs or after `:module` exprs. 2. The frontend inserts `:worldinc` after all struct definitions, method definitions, `using` and `import. 3. The `@eval` macro inserts a worldinc following the call to `eval` if at toplevel 4. A literal (syntactic) call to `include` gains an implicit `worldinc`. Of these the fourth is probably the most questionable, but is necessary to make this non-breaking for most code patterns. Perhaps it would have been better to make `include` a macro from the beginning (esp because it already has semantics that look a little like reaching into the calling module), but that ship has sailed. Unfortunately, I don't see any good intermediate options between this PR and option JuliaLang#3 above. I think option JuliaLang#3 is closest to what we have right now, but if we were to choose it and actually fix the soundness issues, I expect that we would be destroying all performance of global-scope code. For this reason, I would like to try to make the version in this PR work, even if the semantics are a little ugly. The biggest pattern that this PR does not catch is: ``` eval(:(f() = 1)) f() ``` We could apply the same `include` special case to eval, but given the existence of `@eval` which allows addressing this at the macro level, I decided not to. We can decide which way we want to go on this based on what the package ecosystem looks like.
1 parent 9b89e62 commit 27203bb

File tree

6 files changed

+138
-113
lines changed

6 files changed

+138
-113
lines changed

src/abstractinterpretation.jl

Lines changed: 121 additions & 105 deletions
Large diffs are not rendered by default.

src/inferencestate.jl

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,11 @@ to enable flow-sensitive analysis.
209209
"""
210210
const VarTable = Vector{VarState}
211211

212+
struct StatementState
213+
vtypes::Union{VarTable,Nothing}
214+
saw_latestworld::Bool
215+
end
216+
212217
const CACHE_MODE_NULL = 0x00 # not cached, optimization optional
213218
const CACHE_MODE_GLOBAL = 0x01 << 0 # cached globally, optimization required
214219
const CACHE_MODE_LOCAL = 0x01 << 1 # cached locally, optimization required
@@ -260,6 +265,7 @@ mutable struct InferenceState
260265
ssavalue_uses::Vector{BitSet} # ssavalue sparsity and restart info
261266
# TODO: Could keep this sparsely by doing structural liveness analysis ahead of time.
262267
bb_vartables::Vector{Union{Nothing,VarTable}} # nothing if not analyzed yet
268+
bb_saw_latestworld::Vector{Bool}
263269
ssavaluetypes::Vector{Any}
264270
edges::Vector{Any}
265271
stmt_info::Vector{CallInfo}
@@ -320,6 +326,7 @@ mutable struct InferenceState
320326

321327
nslots = length(src.slotflags)
322328
slottypes = Vector{Any}(undef, nslots)
329+
bb_saw_latestworld = Bool[false for i = 1:length(cfg.blocks)]
323330
bb_vartables = Union{Nothing,VarTable}[ nothing for i = 1:length(cfg.blocks) ]
324331
bb_vartable1 = bb_vartables[1] = VarTable(undef, nslots)
325332
argtypes = result.argtypes
@@ -367,7 +374,7 @@ mutable struct InferenceState
367374

368375
this = new(
369376
mi, WorldWithRange(world, valid_worlds), mod, sptypes, slottypes, src, cfg, spec_info,
370-
currbb, currpc, ip, handler_info, ssavalue_uses, bb_vartables, ssavaluetypes, edges, stmt_info,
377+
currbb, currpc, ip, handler_info, ssavalue_uses, bb_vartables, bb_saw_latestworld, ssavaluetypes, edges, stmt_info,
371378
tasks, pclimitations, limitations, cycle_backedges, callstack, parentid, frameid, cycleid,
372379
result, unreachable, bestguess, exc_bestguess, ipo_effects,
373380
restrict_abstract_call_sites, cache_mode, insert_coverage,

src/ssair/irinterp.jl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,19 @@ function abstract_eval_invoke_inst(interp::AbstractInterpreter, inst::Instructio
3838
mi_cache = WorldView(code_cache(interp), world)
3939
code = get(mi_cache, mi, nothing)
4040
code === nothing && return Pair{Any,Tuple{Bool,Bool}}(nothing, (false, false))
41-
argtypes = collect_argtypes(interp, stmt.args[2:end], nothing, irsv)
41+
argtypes = collect_argtypes(interp, stmt.args[2:end], StatementState(nothing, false), irsv)
4242
argtypes === nothing && return Pair{Any,Tuple{Bool,Bool}}(Bottom, (false, false))
4343
return concrete_eval_invoke(interp, code, argtypes, irsv)
4444
end
4545

4646
abstract_eval_ssavalue(s::SSAValue, sv::IRInterpretationState) = abstract_eval_ssavalue(s, sv.ir)
4747

4848
function abstract_eval_phi_stmt(interp::AbstractInterpreter, phi::PhiNode, ::Int, irsv::IRInterpretationState)
49-
return abstract_eval_phi(interp, phi, nothing, irsv)
49+
return abstract_eval_phi(interp, phi, StatementState(nothing, false), irsv)
5050
end
5151

52-
function abstract_call(interp::AbstractInterpreter, arginfo::ArgInfo, irsv::IRInterpretationState)
53-
si = StmtInfo(true) # TODO better job here?
52+
function abstract_call(interp::AbstractInterpreter, arginfo::ArgInfo, sstate::StatementState, irsv::IRInterpretationState)
53+
si = StmtInfo(true, sstate.saw_latestworld) # TODO better job here?
5454
call = abstract_call(interp, arginfo, si, irsv)::Future
5555
Future{Any}(call, interp, irsv) do call, interp, irsv
5656
irsv.ir.stmts[irsv.curridx][:info] = call.info
@@ -147,7 +147,7 @@ function reprocess_instruction!(interp::AbstractInterpreter, inst::Instruction,
147147
if (head === :call || head === :foreigncall || head === :new || head === :splatnew ||
148148
head === :static_parameter || head === :isdefined || head === :boundscheck)
149149
@assert isempty(irsv.tasks) # TODO: this whole function needs to be converted to a stackless design to be a valid AbsIntState, but this should work here for now
150-
result = abstract_eval_statement_expr(interp, stmt, nothing, irsv)
150+
result = abstract_eval_statement_expr(interp, stmt, StatementState(nothing, false), irsv)
151151
reverse!(irsv.tasks)
152152
while true
153153
if length(irsv.callstack) > irsv.frameid
@@ -302,7 +302,7 @@ populate_def_use_map!(tpdum::TwoPhaseDefUseMap, ir::IRCode) =
302302
function is_all_const_call(@nospecialize(stmt), interp::AbstractInterpreter, irsv::IRInterpretationState)
303303
isexpr(stmt, :call) || return false
304304
@inbounds for i = 2:length(stmt.args)
305-
argtype = abstract_eval_value(interp, stmt.args[i], nothing, irsv)
305+
argtype = abstract_eval_value(interp, stmt.args[i], StatementState(nothing, false), irsv)
306306
is_const_argtype(argtype) || return false
307307
end
308308
return true

src/tfuncs.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1426,7 +1426,7 @@ end
14261426
# as well as compute the info for the method matches
14271427
op = unwrapva(argtypes[op_argi])
14281428
v = unwrapva(argtypes[v_argi])
1429-
callinfo = abstract_call(interp, ArgInfo(nothing, Any[op, TF, v]), StmtInfo(true), sv, #=max_methods=#1)
1429+
callinfo = abstract_call(interp, ArgInfo(nothing, Any[op, TF, v]), StmtInfo(true, si.saw_latestworld), sv, #=max_methods=#1)
14301430
TF = Core.Box(TF)
14311431
RT = Core.Box(RT)
14321432
return Future{CallMeta}(callinfo, interp, sv) do callinfo, interp, sv

src/types.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ struct StmtInfo
4141
need thus not be computed.
4242
"""
4343
used::Bool
44+
saw_latestworld::Bool
4445
end
4546

4647
struct SpecInfo

src/validation.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const VALID_EXPR_HEADS = IdDict{Symbol,UnitRange{Int}}(
3939
:using => 1:typemax(Int),
4040
:export => 1:typemax(Int),
4141
:public => 1:typemax(Int),
42+
:latestworld => 0:0,
4243
)
4344

4445
# @enum isn't defined yet, otherwise I'd use it for this

0 commit comments

Comments
 (0)