Skip to content

Commit c07a40f

Browse files
authored
Some small follow-ups to stackless compiler (#55972)
1. Adjust the docstring for `Future`, which had its design changed late in that PR and is now confusing. 2. Add additional assertions validating the API assumptions of the `Future` API. I found it too easy to accidentally misuse this and cause hard-to-debug failures. The biggest change is that `isready` accounts for delayed assignments again, which allows an additional invariant that incomplete tasks must always have other pending tasks, allowing for infinite loop detection in the scheduler. 3. A small fix to use the AbstractInterpreter that created the InferenceState for the callback. We haven't fully defined the semantics of mixed-interpreter inference stacks, but downstream packages were using is and this at least makes it mostly work again.
1 parent b19a7c1 commit c07a40f

File tree

3 files changed

+29
-14
lines changed

3 files changed

+29
-14
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2681,7 +2681,7 @@ function abstract_call(interp::AbstractInterpreter, arginfo::ArgInfo, sv::Infere
26812681
end
26822682
si = StmtInfo(!unused)
26832683
call = abstract_call(interp, arginfo, si, sv)::Future
2684-
Future{Nothing}(call, interp, sv) do call, interp, sv
2684+
Future{Any}(call, interp, sv) do call, interp, sv
26852685
# this only is needed for the side-effect, sequenced before any task tries to consume the return value,
26862686
# which this will do even without returning this Future
26872687
sv.stmt_info[sv.currpc] = call.info
@@ -2833,7 +2833,7 @@ function abstract_eval_new_opaque_closure(interp::AbstractInterpreter, e::Expr,
28332833
pushfirst!(argtypes, rt.env)
28342834
callinfo = abstract_call_opaque_closure(interp, rt,
28352835
ArgInfo(nothing, argtypes), StmtInfo(true), sv, #=check=#false)::Future
2836-
Future{Nothing}(callinfo, interp, sv) do callinfo, interp, sv
2836+
Future{Any}(callinfo, interp, sv) do callinfo, interp, sv
28372837
sv.stmt_info[sv.currpc] = OpaqueClosureCreateInfo(callinfo)
28382838
nothing
28392839
end
@@ -3775,6 +3775,7 @@ function typeinf(interp::AbstractInterpreter, frame::InferenceState)
37753775
takeprev = 0
37763776
while takenext >= frame.frameid
37773777
callee = takenext == 0 ? frame : callstack[takenext]::InferenceState
3778+
interp = callee.interp
37783779
if !isempty(callstack)
37793780
if length(callstack) - frame.frameid >= minwarn
37803781
topmethod = callstack[1].linfo

base/compiler/inferencestate.jl

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,24 +1128,35 @@ end
11281128
"""
11291129
Future{T}
11301130
1131-
Delayed return value for a value of type `T`, similar to RefValue{T}, but
1132-
explicitly represents completed as a `Bool` rather than as `isdefined`.
1133-
Set once with `f[] = v` and accessed with `f[]` afterwards.
1131+
Assign-once delayed return value for a value of type `T`, similar to RefValue{T}.
1132+
Can be constructed in one of three ways:
11341133
1135-
Can also be constructed with the `completed` flag value and a closure to
1136-
produce `x`, as well as the additional arguments to avoid always capturing the
1137-
same couple of values.
1134+
1. With an immediate as `Future{T}(val)`
1135+
2. As an assign-once storage location with `Future{T}()`. Assigned (once) using `f[] = val`.
1136+
3. As a delayed computation with `Future{T}(callback, dep, interp, sv)` to have
1137+
`sv` arrange to call the `callback` with the result of `dep` when it is ready.
1138+
1139+
Use `isready` to check if the value is ready, and `getindex` to get the value.
11381140
"""
11391141
struct Future{T}
11401142
later::Union{Nothing,RefValue{T}}
11411143
now::Union{Nothing,T}
1142-
Future{T}() where {T} = new{T}(RefValue{T}(), nothing)
1144+
function Future{T}() where {T}
1145+
later = RefValue{T}()
1146+
@assert !isassigned(later) "Future{T}() is not allowed for inlinealloc T"
1147+
new{T}(later, nothing)
1148+
end
11431149
Future{T}(x) where {T} = new{T}(nothing, x)
11441150
Future(x::T) where {T} = new{T}(nothing, x)
11451151
end
1146-
isready(f::Future) = f.later === nothing
1152+
isready(f::Future) = f.later === nothing || isassigned(f.later)
11471153
getindex(f::Future{T}) where {T} = (later = f.later; later === nothing ? f.now::T : later[])
1148-
setindex!(f::Future, v) = something(f.later)[] = v
1154+
function setindex!(f::Future, v)
1155+
later = something(f.later)
1156+
@assert !isassigned(later)
1157+
later[] = v
1158+
return f
1159+
end
11491160
convert(::Type{Future{T}}, x) where {T} = Future{T}(x) # support return type conversion
11501161
convert(::Type{Future{T}}, x::Future) where {T} = x::Future{T}
11511162
function Future{T}(f, immediate::Bool, interp::AbstractInterpreter, sv::AbsIntState) where {T}
@@ -1176,7 +1187,6 @@ function Future{T}(f, prev::Future{S}, interp::AbstractInterpreter, sv::AbsIntSt
11761187
end
11771188
end
11781189

1179-
11801190
"""
11811191
doworkloop(args...)
11821192
@@ -1189,12 +1199,16 @@ Each task will be run repeatedly when returning `false`, until it returns `true`
11891199
function doworkloop(interp::AbstractInterpreter, sv::AbsIntState)
11901200
tasks = sv.tasks
11911201
prev = length(tasks)
1202+
prevcallstack = length(sv.callstack)
11921203
prev == 0 && return false
11931204
task = pop!(tasks)
11941205
completed = task(interp, sv)
11951206
tasks = sv.tasks # allow dropping gc root over the previous call
11961207
completed isa Bool || throw(TypeError(:return, "", Bool, task)) # print the task on failure as part of the error message, instead of just "@ workloop:line"
1197-
completed || push!(tasks, task)
1208+
if !completed
1209+
@assert (length(tasks) >= prev || length(sv.callstack) > prevcallstack) "Task did not complete, but also did not create any child tasks"
1210+
push!(tasks, task)
1211+
end
11981212
# efficient post-order visitor: items pushed are executed in reverse post order such
11991213
# that later items are executed before earlier ones, but are fully executed
12001214
# (including any dependencies scheduled by them) before going on to the next item

base/compiler/ssair/irinterp.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ end
5252
function abstract_call(interp::AbstractInterpreter, arginfo::ArgInfo, irsv::IRInterpretationState)
5353
si = StmtInfo(true) # TODO better job here?
5454
call = abstract_call(interp, arginfo, si, irsv)::Future
55-
Future{Nothing}(call, interp, irsv) do call, interp, irsv
55+
Future{Any}(call, interp, irsv) do call, interp, irsv
5656
irsv.ir.stmts[irsv.curridx][:info] = call.info
5757
nothing
5858
end

0 commit comments

Comments
 (0)