Skip to content

Commit

Permalink
bpart: Start enforcing minimum world age for const bparts (#57102)
Browse files Browse the repository at this point in the history
Currently, even though the binding partition system is implemented, it
is largely enabled. New `const` definitions get magically "backdated" to
the first world age in which the binding was undefined.

Additionally, they do not get their own world age and there is currently
no `latestworld` marker after `const` definitions. This PR changes this
situation to give const markers their own world age with appropriate
`latestworld` increments. Both of these are mandatory for `const`
replacement to work.

The other thing this PR does is prepare to remove the automatic "backdating". To
see the difference, consider:

```
function foo($i)
    Core.eval(:(const x = $i))
    x
end
```

Without an intervening world age increment, this will throw an
UndefVarError on this PR. I believe this is the best option for two
reasons:

1. It will allow us infer these to `Union{}` in the future (thus letting
inference prune dead code faster).
2. I think it is less confusing in terms of the world age semantics for
`const` definitions to become active only after they are defined.

To illustrate the second point, suppose we did keep the automatic
backdating. Then we would have:
```
foo(1) => 1
foo(2) => 1
foo(3) => 2
```
as opposed to on this PR:
```
foo(1) => UndefVarError
foo(2) => 1
foo(3) => 2
```

The semantics are consistent, of course, but I am concerned that an
automatic backdating will give users the wrong mental model about world
age, since it "fixes itself" the first time, but if you Revise it, it
will give an unexpected answer. I think this would encourage
accidentally bad code patterns that only break under Revise (where they
are hard to debug).

The counterpoint of course is that that not backdating is a more
breaking choice. As with the rest of the 1.12-era world age semantics
changes, I think taking a look at PkgEval will be helpful.
  • Loading branch information
Keno authored Jan 22, 2025
1 parent 294c4a6 commit 7f99e95
Show file tree
Hide file tree
Showing 36 changed files with 318 additions and 164 deletions.
11 changes: 6 additions & 5 deletions Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3456,10 +3456,10 @@ world_range(ir::IRCode) = ir.valid_worlds
world_range(ci::CodeInfo) = WorldRange(ci.min_world, ci.max_world)
world_range(compact::IncrementalCompact) = world_range(compact.ir)

function force_binding_resolution!(g::GlobalRef)
function force_binding_resolution!(g::GlobalRef, world::UInt)
# Force resolution of the binding
# TODO: This will go away once we switch over to fully partitioned semantics
ccall(:jl_globalref_boundp, Cint, (Any,), g)
ccall(:jl_force_binding_resolution, Cvoid, (Any, Csize_t), g, world)
return nothing
end

Expand All @@ -3477,7 +3477,7 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
# This method is surprisingly hot. For performance, don't ask the runtime to resolve
# the binding unless necessary - doing so triggers an additional lookup, which though
# not super expensive is hot enough to show up in benchmarks.
force_binding_resolution!(g)
force_binding_resolution!(g, min_world(worlds))
return abstract_eval_globalref_type(g, src, false)
end
# return Union{}
Expand All @@ -3490,7 +3490,7 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
end

function lookup_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
force_binding_resolution!(g)
force_binding_resolution!(g, get_inference_world(interp))
partition = lookup_binding_partition(get_inference_world(interp), g)
update_valid_age!(sv, WorldRange(partition.min_world, partition.max_world))
partition
Expand Down Expand Up @@ -3539,7 +3539,8 @@ function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, saw_
partition = abstract_eval_binding_partition!(interp, g, sv)
ret = abstract_eval_partition_load(interp, partition)
if ret.rt !== Union{} && ret.exct === UndefVarError && InferenceParams(interp).assume_bindings_static
if isdefined(g, :binding) && isdefined(g.binding, :value)
b = convert(Core.Binding, g)
if isdefined(b, :value)
ret = RTEffects(ret.rt, Union{}, Effects(generic_getglobal_effects, nothrow=true))
end
# We do not assume in general that assigned global bindings remain assigned.
Expand Down
2 changes: 1 addition & 1 deletion Compiler/src/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
# types of call arguments only once `slot2reg` converts this `IRCode` to the SSA form
# and eliminates slots (see below)
argtypes = sv.slottypes
return IRCode(stmts, sv.cfg, di, argtypes, meta, sv.sptypes, WorldRange(ci.min_world, ci.max_world))
return IRCode(stmts, sv.cfg, di, argtypes, meta, sv.sptypes, world_range(ci))
end

function process_meta!(meta::Vector{Expr}, @nospecialize stmt)
Expand Down
4 changes: 2 additions & 2 deletions Compiler/src/ssair/ir.jl
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ struct IRCode
function IRCode(stmts::InstructionStream, cfg::CFG, debuginfo::DebugInfoStream,
argtypes::Vector{Any}, meta::Vector{Expr}, sptypes::Vector{VarState},
valid_worlds=WorldRange(typemin(UInt), typemax(UInt)))
return new(stmts, argtypes, sptypes, debuginfo, cfg, NewNodeStream(), meta)
return new(stmts, argtypes, sptypes, debuginfo, cfg, NewNodeStream(), meta, valid_worlds)
end
function IRCode(ir::IRCode, stmts::InstructionStream, cfg::CFG, new_nodes::NewNodeStream)
di = ir.debuginfo
Expand Down Expand Up @@ -1462,7 +1462,7 @@ function process_node!(compact::IncrementalCompact, result_idx::Int, inst::Instr
result[result_idx][:stmt] = GotoNode(label)
result_idx += 1
elseif isa(stmt, GlobalRef)
total_flags = IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE
total_flags = IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW
flag = result[result_idx][:flag]
if has_flag(flag, total_flags)
ssa_rename[idx] = stmt
Expand Down
2 changes: 1 addition & 1 deletion Compiler/src/ssair/legacy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function inflate_ir!(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{A
di = DebugInfoStream(nothing, ci.debuginfo, nstmts)
stmts = InstructionStream(code, ssavaluetypes, info, di.codelocs, ci.ssaflags)
meta = Expr[]
return IRCode(stmts, cfg, di, argtypes, meta, sptypes, WorldRange(ci.min_world, ci.max_world))
return IRCode(stmts, cfg, di, argtypes, meta, sptypes, world_range(ci))
end

"""
Expand Down
12 changes: 9 additions & 3 deletions Compiler/src/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ end

if !isdefined(@__MODULE__, Symbol("@verify_error"))
macro verify_error(arg)
arg isa String && return esc(:(print && println(stderr, $arg)))
arg isa String && return esc(:(print && println($(GlobalRef(Core, :stderr)), $arg)))
isexpr(arg, :string) || error("verify_error macro expected a string expression")
pushfirst!(arg.args, GlobalRef(Core, :stderr))
pushfirst!(arg.args, :println)
Expand Down Expand Up @@ -61,8 +61,14 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int,
raise_error()
end
elseif isa(op, GlobalRef)
if !isdefined(op.mod, op.name) || !isconst(op.mod, op.name)
@verify_error "Unbound GlobalRef not allowed in value position"
force_binding_resolution!(op, min_world(ir.valid_worlds))
bpart = lookup_binding_partition(min_world(ir.valid_worlds), op)
while is_some_imported(binding_kind(bpart)) && max_world(ir.valid_worlds) <= bpart.max_world
imported_binding = partition_restriction(bpart)::Core.Binding
bpart = lookup_binding_partition(min_world(ir.valid_worlds), imported_binding)
end
if !is_defined_const_binding(binding_kind(bpart)) || (bpart.max_world < max_world(ir.valid_worlds))
@verify_error "Unbound or partitioned GlobalRef not allowed in value position"
raise_error()
end
elseif isa(op, Expr)
Expand Down
2 changes: 1 addition & 1 deletion Compiler/src/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ function _getfield_tfunc_const(@nospecialize(sv), name::Const)
if isa(sv, DataType) && nv == DATATYPE_TYPES_FIELDINDEX && isdefined(sv, nv)
return Const(getfield(sv, nv))
end
if isconst(typeof(sv), nv)
if !isa(sv, Module) && isconst(typeof(sv), nv)
if isdefined(sv, nv)
return Const(getfield(sv, nv))
end
Expand Down
4 changes: 2 additions & 2 deletions Compiler/test/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ end

(src, _) = only(code_typed(sum27403, Tuple{Vector{Int}}))
@test !any(src.code) do x
x isa Expr && x.head === :invoke && x.args[2] !== Core.GlobalRef(Base, :throw_boundserror)
x isa Expr && x.head === :invoke && !(x.args[2] in (Core.GlobalRef(Base, :throw_boundserror), Base.throw_boundserror))
end
end

Expand Down Expand Up @@ -313,7 +313,7 @@ end
const _a_global_array = [1]
f_inline_global_getindex() = _a_global_array[1]
let ci = code_typed(f_inline_global_getindex, Tuple{})[1].first
@test any(x->(isexpr(x, :call) && x.args[1] === GlobalRef(Base, :memoryrefget)), ci.code)
@test any(x->(isexpr(x, :call) && x.args[1] in (GlobalRef(Base, :memoryrefget), Base.memoryrefget)), ci.code)
end

# Issue #29114 & #36087 - Inlining of non-tuple splats
Expand Down
1 change: 1 addition & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ end
# we know whether the .ji can just give the Base copy or not.
# TODO: We may want to do this earlier to avoid TOCTOU issues.
const _compiler_require_dependencies = Any[]
@Core.latestworld
for i = 1:length(_included_files)
isassigned(_included_files, i) || continue
(mod, file) = _included_files[i]
Expand Down
12 changes: 6 additions & 6 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ function exec_options(opts)
elseif cmd == 'm'
entrypoint = push!(split(arg, "."), "main")
Base.eval(Main, Expr(:import, Expr(:., Symbol.(entrypoint)...)))
if !should_use_main_entrypoint()
if !invokelatest(should_use_main_entrypoint)
error("`main` in `$arg` not declared as entry point (use `@main` to do so)")
end
return false
Expand Down Expand Up @@ -408,8 +408,7 @@ function load_InteractiveUtils(mod::Module=Main)
return nothing
end
end
Core.eval(mod, :(using Base.MainInclude.InteractiveUtils))
return MainInclude.InteractiveUtils
return Core.eval(mod, :(using Base.MainInclude.InteractiveUtils; Base.MainInclude.InteractiveUtils))
end

function load_REPL()
Expand Down Expand Up @@ -556,11 +555,12 @@ function _start()
local ret = 0
try
repl_was_requested = exec_options(JLOptions())
if should_use_main_entrypoint() && !is_interactive
if invokelatest(should_use_main_entrypoint) && !is_interactive
main = invokelatest(getglobal, Main, :main)
if Base.generating_output()
precompile(Main.main, (typeof(ARGS),))
precompile(main, (typeof(ARGS),))
else
ret = invokelatest(Main.main, ARGS)
ret = invokelatest(main, ARGS)
end
elseif (repl_was_requested || is_interactive)
# Run the Base `main`, which will either load the REPL stdlib
Expand Down
6 changes: 6 additions & 0 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,9 @@ arbitrary code in fixed worlds. `world` may be `UnitRange`, in which case the ma
will error unless the binding is valid and has the same value across the entire world
range.
As a special case, the world `∞` always refers to the latest world, even if that world
is newer than the world currently running.
The `@world` macro is primarily used in the printing of bindings that are no longer
available in the current world.
Expand All @@ -1290,6 +1293,9 @@ julia> fold
This functionality requires at least Julia 1.12.
"""
macro world(sym, world)
if world == :∞
world = Expr(:call, get_world_counter)
end
if isa(sym, Symbol)
return :($(_resolve_in_world)($(esc(world)), $(QuoteNode(GlobalRef(__module__, sym)))))
elseif isa(sym, GlobalRef)
Expand Down
22 changes: 13 additions & 9 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2364,6 +2364,18 @@ function require(into::Module, mod::Symbol)
return invoke_in_world(world, __require, into, mod)
end

function check_for_hint(into, mod)
return begin
if isdefined(into, mod) && getfield(into, mod) isa Module
true, "."
elseif isdefined(parentmodule(into), mod) && getfield(parentmodule(into), mod) isa Module
true, ".."
else
false, ""
end
end
end

function __require(into::Module, mod::Symbol)
if into === __toplevel__ && generating_output(#=incremental=#true)
error("`using/import $mod` outside of a Module detected. Importing a package outside of a module \
Expand All @@ -2377,15 +2389,7 @@ function __require(into::Module, mod::Symbol)
if uuidkey_env === nothing
where = PkgId(into)
if where.uuid === nothing
hint, dots = begin
if isdefined(into, mod) && getfield(into, mod) isa Module
true, "."
elseif isdefined(parentmodule(into), mod) && getfield(parentmodule(into), mod) isa Module
true, ".."
else
false, ""
end
end
hint, dots = invokelatest(check_for_hint, into, mod)
hint_message = hint ? ", maybe you meant `import/using $(dots)$(mod)`" : ""
install_message = if mod != :Pkg
start_sentence = hint ? "Otherwise, run" : "Run"
Expand Down
4 changes: 2 additions & 2 deletions base/logging/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ function logmsg_code(_module, file, line, level, message, exs...)
kwargs = (;$(log_data.kwargs...))
true
else
@invokelatest logging_error(logger, level, _module, group, id, file, line, err, false)
@invokelatest $(logging_error)(logger, level, _module, group, id, file, line, err, false)
false
end
end
Expand All @@ -384,7 +384,7 @@ function logmsg_code(_module, file, line, level, message, exs...)
kwargs = (;$(log_data.kwargs...))
true
catch err
@invokelatest logging_error(logger, level, _module, group, id, file, line, err, true)
@invokelatest $(logging_error)(logger, level, _module, group, id, file, line, err, true)
false
end
end
Expand Down
77 changes: 56 additions & 21 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,18 @@ macro invoke(ex)
return esc(out)
end

apply_gr(gr::GlobalRef, @nospecialize args...) = getglobal(gr.mod, gr.name)(args...)
apply_gr_kw(@nospecialize(kwargs::NamedTuple), gr::GlobalRef, @nospecialize args...) = Core.kwcall(kwargs, getglobal(gr.mod, gr.name), args...)

function invokelatest_gr(gr::GlobalRef, @nospecialize args...; kwargs...)
@inline
kwargs = merge(NamedTuple(), kwargs)
if isempty(kwargs)
return Core._call_latest(apply_gr, gr, args...)
end
return Core._call_latest(apply_gr_kw, kwargs, gr, args...)
end

"""
@invokelatest f(args...; kwargs...)

Expand All @@ -1375,22 +1387,11 @@ It also supports the following syntax:
- `@invokelatest xs[i]` expands to `Base.invokelatest(getindex, xs, i)`
- `@invokelatest xs[i] = v` expands to `Base.invokelatest(setindex!, xs, v, i)`

```jldoctest
julia> @macroexpand @invokelatest f(x; kw=kwv)
:(Base.invokelatest(f, x; kw = kwv))

julia> @macroexpand @invokelatest x.f
:(Base.invokelatest(Base.getproperty, x, :f))

julia> @macroexpand @invokelatest x.f = v
:(Base.invokelatest(Base.setproperty!, x, :f, v))

julia> @macroexpand @invokelatest xs[i]
:(Base.invokelatest(Base.getindex, xs, i))

julia> @macroexpand @invokelatest xs[i] = v
:(Base.invokelatest(Base.setindex!, xs, v, i))
```
!!! note
If `f` is a global, it will be resolved consistently
in the (latest) world as the call target. However, all other arguments
(as well as `f` itself if it is not a literal global) will be evaluated
in the current world age.

!!! compat "Julia 1.7"
This macro requires Julia 1.7 or later.
Expand All @@ -1404,11 +1405,45 @@ julia> @macroexpand @invokelatest xs[i] = v
macro invokelatest(ex)
topmod = _topmod(__module__)
f, args, kwargs = destructure_callex(topmod, ex)
out = Expr(:call, GlobalRef(Base, :invokelatest))
isempty(kwargs) || push!(out.args, Expr(:parameters, kwargs...))
push!(out.args, f)
append!(out.args, args)
return esc(out)

if !isa(f, GlobalRef)
out_f = Expr(:call, GlobalRef(Base, :invokelatest))
isempty(kwargs) || push!(out_f.args, Expr(:parameters, kwargs...))

if isexpr(f, :(.))
s = gensym()
check = quote
$s = $(f.args[1])
isa($s, Module)
end
push!(out_f.args, Expr(:(.), s, f.args[2]))
else
push!(out_f.args, f)
end
append!(out_f.args, args)

if @isdefined(s)
f = :(GlobalRef($s, $(f.args[2])))
elseif !isa(f, Symbol)
return esc(out_f)
else
check = :($(Expr(:isglobal, f)))
end
end

out_gr = Expr(:call, GlobalRef(Base, :invokelatest_gr))
isempty(kwargs) || push!(out_gr.args, Expr(:parameters, kwargs...))
push!(out_gr.args, isa(f, GlobalRef) ? QuoteNode(f) :
isa(f, Symbol) ? QuoteNode(GlobalRef(__module__, f)) :
f)
append!(out_gr.args, args)

if isa(f, GlobalRef)
return esc(out_gr)
end

# f::Symbol
return esc(:($check ? $out_gr : $out_f))
end

function destructure_callex(topmod::Module, @nospecialize(ex))
Expand Down
16 changes: 12 additions & 4 deletions base/runtime_internals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -239,12 +239,16 @@ function lookup_binding_partition(world::UInt, b::Core.Binding)
ccall(:jl_get_binding_partition, Ref{Core.BindingPartition}, (Any, UInt), b, world)
end

function lookup_binding_partition(world::UInt, gr::Core.GlobalRef)
function convert(::Type{Core.Binding}, gr::Core.GlobalRef)
if isdefined(gr, :binding)
b = gr.binding
return gr.binding
else
b = ccall(:jl_get_module_binding, Ref{Core.Binding}, (Any, Any, Cint), gr.mod, gr.name, true)
return ccall(:jl_get_module_binding, Ref{Core.Binding}, (Any, Any, Cint), gr.mod, gr.name, true)
end
end

function lookup_binding_partition(world::UInt, gr::Core.GlobalRef)
b = convert(Core.Binding, gr)
return lookup_binding_partition(world, b)
end

Expand Down Expand Up @@ -420,7 +424,11 @@ end
"""
isconst(t::DataType, s::Union{Int,Symbol}) -> Bool

Determine whether a field `s` is declared `const` in a given type `t`.
Determine whether a field `s` is const in a given type `t`
in the sense that a read from said field is consistent
for egal objects. Note in particular that out-of-bounds
fields are considered const under this definition (because
they always throw).
"""
function isconst(@nospecialize(t::Type), s::Symbol)
@_foldable_meta
Expand Down
Loading

0 comments on commit 7f99e95

Please sign in to comment.