Skip to content

Commit df980e9

Browse files
authored
bpart: Fully switch to partitioned semantics (JuliaLang#57253)
This is the final PR in the binding partitions series (modulo bugs and tweaks), i.e. it closes JuliaLang#54654 and thus closes JuliaLang#40399, which was the original design sketch. This thus activates the full designed semantics for binding partitions, in particular allowing safe replacement of const bindings. It in particular allows struct redefinitions. This thus closes timholy/Revise.jl#18 and also closes JuliaLang#38584. The biggest semantic change here is probably that this gets rid of the notion of "resolvedness" of a binding. Previously, a lot of the behavior of our implementation depended on when bindings were "resolved", which could happen at basically an arbitrary point (in the compiler, in REPL completion, in a different thread), making a lot of the semantics around bindings ill- or at least implementation-defined. There are several related issues in the bugtracker, so this closes JuliaLang#14055 closes JuliaLang#44604 closes JuliaLang#46354 closes JuliaLang#30277 It is also the last step to close JuliaLang#24569. It also supports bindings for undef->defined transitions and thus closes JuliaLang#53958 closes JuliaLang#54733 - however, this is not activated yet for performance reasons and may need some further optimization. Since resolvedness no longer exists, we need to replace it with some hopefully more well-defined semantics. I will describe the semantics below, but before I do I will make two notes: 1. There are a number of cases where these semantics will behave slightly differently than the old semantics absent some other task going around resolving random bindings. 2. The new behavior (except for the replacement stuff) was generally permissible under the old semantics if the bindings happened to be resolved at the right time. With all that said, there are essentially three "strengths" of bindings: 1. Implicit Bindings: Anything implicitly obtained from `using Mod`, "no binding", plus slightly more exotic corner cases around conflicts 2. Weakly declared bindings: Declared using `global sym` and nothing else 3. Strongly declared bindings: Declared using `global sym::T`, `const sym=val`, `import Mod: sym`, `using Mod: sym` or as an implicit strong global declaration in `sym=val`, where `sym` is known to be global (either by being at toplevle or as `global sym=val` inside a function). In general, you always allowed to syntactically replace a weaker binding by a stronger one (although the runtime permits arbitrary binding deletion now, this is just a syntactic constraint to catch errors). Second, any implicit binding can be replaced by other implicit bindings as the result of changing the `using`'ed module. And lastly, any constants may be replaced by any other constants (irrespective of type). We do not currently allow replacing globals, but may consider changing that in 1.13. This is mostly how things used to work, as well in the absence of any stray external binding resolutions. The most prominent difference is probably this one: ``` set_foo!() = global foo = 1 ``` In the above terminology, this now always declares a "strongly declared binding", whereas before it declared a "weakly declared binding" that would become strongly declared on first write to the global (unless of course somebody had created a different strongly declared global in the meantime). To see the difference, this is now disallowed: ``` julia> set_foo!() = global foo = 1 set_foo! (generic function with 1 method) julia> const foo = 1 ERROR: cannot declare Main.foo constant; it was already declared global Stacktrace: [1] top-level scope @ REPL[2]:1 ``` Before it would depend on the order of binding resolution (although it just crashes on current master for some reason - whoops, probably my fault). Another major change is the ambiguousness of imports. In: ``` module M1; export x; x=1; end module M2; export x; x=2; end using .M1, .M2 ``` the binding `Main.x` is now always ambiguous and will throw on access. Before which binding you get, would depend on resolution order. To choose one, use an explicit import (which was the behavior you would previously get if neither binding was resolved before both imports).
1 parent 26af553 commit df980e9

File tree

7 files changed

+28
-44
lines changed

7 files changed

+28
-44
lines changed

src/Compiler.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ using Core: ABIOverride, Builtin, CodeInstance, IntrinsicFunction, MethodInstanc
4949

5050
using Base
5151
using Base: @_foldable_meta, @_gc_preserve_begin, @_gc_preserve_end, @nospecializeinfer,
52-
BINDING_KIND_GLOBAL, BINDING_KIND_UNDEF_CONST, BINDING_KIND_BACKDATED_CONST,
52+
BINDING_KIND_GLOBAL, BINDING_KIND_UNDEF_CONST, BINDING_KIND_BACKDATED_CONST, BINDING_KIND_DECLARED,
5353
Base, BitVector, Bottom, Callable, DataTypeFieldDesc,
5454
EffectsOverride, Filter, Generator, IteratorSize, JLOptions, NUM_EFFECTS_OVERRIDES,
5555
OneTo, Ordering, RefValue, SizeUnknown, _NAMEDTUPLE_NAME,

src/abstractinterpretation.jl

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3464,14 +3464,7 @@ world_range(ci::CodeInfo) = WorldRange(ci.min_world, ci.max_world)
34643464
world_range(ci::CodeInstance) = WorldRange(ci.min_world, ci.max_world)
34653465
world_range(compact::IncrementalCompact) = world_range(compact.ir)
34663466

3467-
function force_binding_resolution!(g::GlobalRef, world::UInt)
3468-
# Force resolution of the binding
3469-
# TODO: This will go away once we switch over to fully partitioned semantics
3470-
ccall(:jl_force_binding_resolution, Cvoid, (Any, Csize_t), g, world)
3471-
return nothing
3472-
end
3473-
3474-
function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode, IncrementalCompact}, retry_after_resolve::Bool=true)
3467+
function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode, IncrementalCompact})
34753468
worlds = world_range(src)
34763469
partition = lookup_binding_partition(min_world(worlds), g)
34773470
partition.max_world < max_world(worlds) && return Any
@@ -3480,25 +3473,18 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
34803473
partition = lookup_binding_partition(min_world(worlds), imported_binding)
34813474
partition.max_world < max_world(worlds) && return Any
34823475
end
3483-
if is_some_guard(binding_kind(partition))
3484-
if retry_after_resolve
3485-
# This method is surprisingly hot. For performance, don't ask the runtime to resolve
3486-
# the binding unless necessary - doing so triggers an additional lookup, which though
3487-
# not super expensive is hot enough to show up in benchmarks.
3488-
force_binding_resolution!(g, min_world(worlds))
3489-
return abstract_eval_globalref_type(g, src, false)
3490-
end
3476+
kind = binding_kind(partition)
3477+
if is_some_guard(kind)
34913478
# return Union{}
34923479
return Any
34933480
end
3494-
if is_some_const_binding(binding_kind(partition))
3481+
if is_some_const_binding(kind)
34953482
return Const(partition_restriction(partition))
34963483
end
3497-
return partition_restriction(partition)
3484+
return kind == BINDING_KIND_DECLARED ? Any : partition_restriction(partition)
34983485
end
34993486

35003487
function lookup_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
3501-
force_binding_resolution!(g, get_inference_world(interp))
35023488
partition = lookup_binding_partition(get_inference_world(interp), g)
35033489
update_valid_age!(sv, WorldRange(partition.min_world, partition.max_world))
35043490
partition
@@ -3541,7 +3527,11 @@ function abstract_eval_partition_load(interp::AbstractInterpreter, partition::Co
35413527
return RTEffects(rt, Union{}, Effects(EFFECTS_TOTAL, inaccessiblememonly=is_mutation_free_argtype(rt) ? ALWAYS_TRUE : ALWAYS_FALSE))
35423528
end
35433529

3544-
rt = partition_restriction(partition)
3530+
if kind == BINDING_KIND_DECLARED
3531+
rt = Any
3532+
else
3533+
rt = partition_restriction(partition)
3534+
end
35453535
return RTEffects(rt, UndefVarError, generic_getglobal_effects)
35463536
end
35473537

@@ -3580,7 +3570,7 @@ function global_assignment_binding_rt_exct(interp::AbstractInterpreter, partitio
35803570
elseif is_some_const_binding(kind)
35813571
return Pair{Any,Any}(Bottom, ErrorException)
35823572
end
3583-
ty = partition_restriction(partition)
3573+
ty = kind == BINDING_KIND_DECLARED ? Any : partition_restriction(partition)
35843574
wnewty = widenconst(newty)
35853575
if !hasintersect(wnewty, ty)
35863576
return Pair{Any,Any}(Bottom, TypeError)

src/ssair/ir.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ function is_relevant_expr(e::Expr)
581581
:foreigncall, :isdefined, :copyast,
582582
:throw_undef_if_not,
583583
:cfunction, :method, :pop_exception,
584-
:leave,
584+
:leave, :const, :globaldecl,
585585
:new_opaque_closure)
586586
end
587587

src/ssair/verify.jl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int,
6161
raise_error()
6262
end
6363
elseif isa(op, GlobalRef)
64-
force_binding_resolution!(op, min_world(ir.valid_worlds))
6564
bpart = lookup_binding_partition(min_world(ir.valid_worlds), op)
6665
while is_some_imported(binding_kind(bpart)) && max_world(ir.valid_worlds) <= bpart.max_world
6766
imported_binding = partition_restriction(bpart)::Core.Binding

src/validation.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const VALID_EXPR_HEADS = IdDict{Symbol,UnitRange{Int}}(
2222
:copyast => 1:1,
2323
:meta => 0:typemax(Int),
2424
:global => 1:1,
25-
:globaldecl => 2:2,
25+
:globaldecl => 1:2,
2626
:foreigncall => 5:typemax(Int), # name, RT, AT, nreq, (cconv, effects), args..., roots...
2727
:cfunction => 5:5,
2828
:isdefined => 1:2,

test/effects.jl

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -378,32 +378,27 @@ let effects = Base.infer_effects(; optimize=false) do
378378
end
379379

380380
# we should taint `nothrow` if the binding doesn't exist and isn't fixed yet,
381-
# as the cached effects can be easily wrong otherwise
382-
# since the inference currently doesn't track "world-age" of global variables
383-
@eval global_assignment_undefinedyet() = $(GlobalRef(@__MODULE__, :UNDEFINEDYET)) = 42
384381
setglobal!_nothrow_undefinedyet() = setglobal!(@__MODULE__, :UNDEFINEDYET, 42)
385-
let effects = Base.infer_effects() do
386-
global_assignment_undefinedyet()
387-
end
382+
let effects = Base.infer_effects(setglobal!_nothrow_undefinedyet)
388383
@test !Compiler.is_nothrow(effects)
389384
end
390-
let effects = Base.infer_effects() do
391-
setglobal!_nothrow_undefinedyet()
392-
end
393-
@test !Compiler.is_nothrow(effects)
385+
@test_throws ErrorException setglobal!_nothrow_undefinedyet()
386+
# This declares the binding as ::Any
387+
@eval global_assignment_undefinedyet() = $(GlobalRef(@__MODULE__, :UNDEFINEDYET)) = 42
388+
let effects = Base.infer_effects(global_assignment_undefinedyet)
389+
@test Compiler.is_nothrow(effects)
394390
end
395-
global UNDEFINEDYET::String = "0"
396-
let effects = Base.infer_effects() do
397-
global_assignment_undefinedyet()
398-
end
391+
# Again with type mismatch
392+
global UNDEFINEDYET2::String = "0"
393+
setglobal!_nothrow_undefinedyet2() = setglobal!(@__MODULE__, :UNDEFINEDYET2, 42)
394+
@eval global_assignment_undefinedyet2() = $(GlobalRef(@__MODULE__, :UNDEFINEDYET2)) = 42
395+
let effects = Base.infer_effects(global_assignment_undefinedyet2)
399396
@test !Compiler.is_nothrow(effects)
400397
end
401-
let effects = Base.infer_effects() do
402-
setglobal!_nothrow_undefinedyet()
403-
end
398+
let effects = Base.infer_effects(setglobal!_nothrow_undefinedyet2)
404399
@test !Compiler.is_nothrow(effects)
405400
end
406-
@test_throws Union{ErrorException,TypeError} setglobal!_nothrow_undefinedyet() # TODO: what kind of error should this be?
401+
@test_throws TypeError setglobal!_nothrow_undefinedyet2()
407402

408403
# Nothrow for setfield!
409404
mutable struct SetfieldNothrow

test/inference.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6160,7 +6160,7 @@ end === Int
61606160
swapglobal!(@__MODULE__, :swapglobal!_xxx, x)
61616161
end === Union{}
61626162

6163-
global swapglobal!_must_throw
6163+
eval(Expr(:const, :swapglobal!_must_throw))
61646164
@newinterp SwapGlobalInterp
61656165
Compiler.InferenceParams(::SwapGlobalInterp) = Compiler.InferenceParams(; assume_bindings_static=true)
61666166
function func_swapglobal!_must_throw(x)

0 commit comments

Comments
 (0)