Skip to content

Commit

Permalink
WIP: Make world-age increments explicit
Browse files Browse the repository at this point in the history
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 #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 #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 in 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 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 #3 above. I think option #3 is closes 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.
  • Loading branch information
Keno committed Nov 9, 2024
1 parent 8593792 commit d085375
Show file tree
Hide file tree
Showing 24 changed files with 293 additions and 171 deletions.
6 changes: 5 additions & 1 deletion base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -259,18 +259,21 @@ else
const UInt = UInt32
end

function iterate end
function Typeof end
ccall(:jl_toplevel_eval_in, Any, (Any, Any),
Core, quote
(f::typeof(Typeof))(x) = ($(_expr(:meta,:nospecialize,:x)); isa(x,Type) ? Type{x} : typeof(x))
end)

function iterate end

macro nospecialize(x)
_expr(:meta, :nospecialize, x)
end
Expr(@nospecialize args...) = _expr(args...)

macro worldinc() Expr(:worldinc) end

_is_internal(__module__) = __module__ === Core
# can be used in place of `@assume_effects :total` (supposed to be used for bootstrapping)
macro _total_meta()
Expand Down Expand Up @@ -518,6 +521,7 @@ eval(Core, quote
InterConditional(slot::Int, @nospecialize(thentype), @nospecialize(elsetype)) = $(Expr(:new, :InterConditional, :slot, :thentype, :elsetype))
MethodMatch(@nospecialize(spec_types), sparams::SimpleVector, method::Method, fully_covers::Bool) = $(Expr(:new, :MethodMatch, :spec_types, :sparams, :method, :fully_covers))
end)
@worldinc

const NullDebugInfo = DebugInfo(:none)

Expand Down
14 changes: 12 additions & 2 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,20 @@ Evaluate an expression with values interpolated into it using `eval`.
If two arguments are provided, the first is the module to evaluate in.
"""
macro eval(ex)
return Expr(:escape, Expr(:call, GlobalRef(Core, :eval), __module__, Expr(:quote, ex)))
g = ccall(:jl_gensym, Ref{Symbol}, ())
return Expr(:let, Expr(:(=), g,
Expr(:escape, Expr(:call, GlobalRef(Core, :eval), __module__, Expr(:quote, ex)))),
Expr(:block,
Expr(:var"worldinc-if-toplevel"),
g))
end
macro eval(mod, ex)
return Expr(:escape, Expr(:call, GlobalRef(Core, :eval), mod, Expr(:quote, ex)))
g = ccall(:jl_gensym, Ref{Symbol}, ())
return Expr(:let, Expr(:(=), g,
Expr(:escape, Expr(:call, GlobalRef(Core, :eval), mod, Expr(:quote, ex)))),
Expr(:block,
Expr(:var"worldinc-if-toplevel"),
g))
end

# use `@eval` here to directly form `:new` expressions avoid implicit `convert`s
Expand Down
8 changes: 8 additions & 0 deletions base/sysimg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ using .Base

# Set up Main module
using Base.MainInclude # ans, err, and sometimes Out
Core.@worldinc

# These definitions calls Base._include rather than Base.include to get
# one-frame stacktraces for the common case of using include(fname) in Main.
Expand All @@ -29,6 +30,13 @@ actually evaluates `mapexpr(expr)`. If it is omitted, `mapexpr` defaults to [`i
Use [`Base.include`](@ref) to evaluate a file into another module.
!!! note
Julia's syntax lowering recognizes an explicit call to a literal `include`
at top-level and inserts an implicit `@Core.worldinc` to make any include'd
definitions visible to subsequent code. Note however that this recognition
is *syntactic*. I.e. assigning `const myinclude = include` may require
and explicit `@Core.worldinc` call after `myinclude`.
!!! compat "Julia 1.5"
Julia 1.5 is required for passing the `mapexpr` argument.
"""
Expand Down
2 changes: 1 addition & 1 deletion base/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ end

function _setindex(v, i::Integer, args::Vararg{Any,N}) where {N}
@inline
return ntuple(j -> ifelse(j == i, v, args[j]), Val{N}())
return ntuple(j -> ifelse(j == i, v, args[j]), Val{N}())::NTuple{N, Any}
end


Expand Down
2 changes: 2 additions & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ JL_DLLEXPORT jl_sym_t *jl_release_sym;
JL_DLLEXPORT jl_sym_t *jl_acquire_release_sym;
JL_DLLEXPORT jl_sym_t *jl_sequentially_consistent_sym;
JL_DLLEXPORT jl_sym_t *jl_uninferred_sym;
JL_DLLEXPORT jl_sym_t *jl_worldinc_sym;

static const uint8_t flisp_system_image[] = {
#include <julia_flisp.boot.inc>
Expand Down Expand Up @@ -461,6 +462,7 @@ void jl_init_common_symbols(void)
jl_acquire_release_sym = jl_symbol("acquire_release");
jl_sequentially_consistent_sym = jl_symbol("sequentially_consistent");
jl_uninferred_sym = jl_symbol("uninferred");
jl_worldinc_sym = jl_symbol("worldinc");
}

JL_DLLEXPORT void jl_lisp_prompt(void)
Expand Down
8 changes: 3 additions & 5 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,6 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
s->ip = ip;
if (ip >= ns)
jl_error("`body` expression must terminate in `return`. Use `block` instead.");
if (toplevel)
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
jl_value_t *stmt = jl_array_ptr_ref(stmts, ip);
assert(!jl_is_phinode(stmt));
size_t next_ip = ip + 1;
Expand Down Expand Up @@ -643,6 +641,9 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
jl_eval_const_decl(s->module, jl_exprarg(stmt, 0), val);
s->locals[jl_source_nslots(s->src) + s->ip] = jl_nothing;
}
else if (head == jl_worldinc_sym) {
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
}
else if (jl_is_toplevel_only_expr(stmt)) {
jl_toplevel_eval(s->module, stmt);
}
Expand Down Expand Up @@ -887,10 +888,7 @@ jl_value_t *NOINLINE jl_interpret_toplevel_thunk(jl_module_t *m, jl_code_info_t
s->mi = NULL;
s->ci = NULL;
JL_GC_ENABLEFRAME(s);
jl_task_t *ct = jl_current_task;
size_t last_age = ct->world_age;
jl_value_t *r = eval_body(stmts, s, 0, 1);
ct->world_age = last_age;
JL_GC_POP();
return r;
}
Expand Down
2 changes: 1 addition & 1 deletion src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@

(define (toplevel-only-expr? e)
(and (pair? e)
(or (memq (car e) '(toplevel line module import using export public
(or (memq (car e) '(toplevel line module export public
error incomplete))
(and (memq (car e) '(global const)) (every symbol? (cdr e))))))

Expand Down
44 changes: 37 additions & 7 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,7 @@
;; otherwise do an assignment to trigger an error
(const (globalref (thismodule) ,name) ,name)))
(const (globalref (thismodule) ,name) ,name))
(worldinc)
(call (core _typebody!) ,name (call (core svec) ,@field-types))
(null)))
;; "inner" constructors
Expand Down Expand Up @@ -1076,6 +1077,7 @@
(call (core _equiv_typedef) (globalref (thismodule) ,name) ,name))
(null)
(const (globalref (thismodule) ,name) ,name))
(worldinc)
(null))))))

(define (primitive-type-def-expr n name params super)
Expand All @@ -1096,6 +1098,7 @@
(call (core _equiv_typedef) (globalref (thismodule) ,name) ,name))
(null)
(const (globalref (thismodule) ,name) ,name))
(worldinc)
(null))))))

;; take apart a type signature, e.g. T{X} <: S{Y}
Expand Down Expand Up @@ -1210,7 +1213,7 @@
(cond ((and (length= e 2) (or (symbol? name) (globalref? name)))
(if (not (valid-name? name))
(error (string "invalid function name \"" name "\"")))
`(method ,name))
`(block (method ,name) (worldinc) (unnecessary ,name)))
((not (pair? name)) e)
((eq? (car name) 'call)
(let* ((raw-typevars (or where '()))
Expand Down Expand Up @@ -2733,6 +2736,9 @@
((and (eq? (identifier-name f) '^) (length= e 4) (integer? (cadddr e)))
(expand-forms
`(call (top literal_pow) ,f ,(caddr e) (call (call (core apply_type) (top Val) ,(cadddr e))))))
((eq? f 'include)
(let ((r (make-ssavalue)))
`(block (= ,r ,(map expand-forms e)) (worldinc-if-toplevel) ,r)))
(else
(map expand-forms e))))
(map expand-forms e)))
Expand Down Expand Up @@ -4114,15 +4120,17 @@ f(x) = yt(x)
`(lambda ,(cadr lam2)
(,(clear-capture-bits (car vis))
,@(cdr vis))
,body)))))
,body)))
(worldinc)))
(else
(let* ((exprs (lift-toplevel (convert-lambda lam2 '|#anon| #t '() #f parsed-method-stack)))
(top-stmts (cdr exprs))
(newlam (compact-and-renumber (linearize (car exprs)) 'none 0)))
`(toplevel-butfirst
(block ,@sp-inits
(method ,(cadr e) ,(cl-convert sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)
,(julia-bq-macro newlam)))
,(julia-bq-macro newlam))
(worldinc))
,@top-stmts))))

;; local case - lift to a new type at top level
Expand Down Expand Up @@ -4261,15 +4269,17 @@ f(x) = yt(x)
`(toplevel-butfirst
(null)
,@sp-inits
,@mk-method)
,@mk-method
(worldinc))
(begin
(put! defined name #t)
`(toplevel-butfirst
,(convert-assignment name mk-closure fname lam interp opaq parsed-method-stack globals locals)
,@typedef
,@(map (lambda (v) `(moved-local ,v)) moved-vars)
,@sp-inits
,@mk-method))))))))
,@mk-method
(worldinc)))))))))
((lambda) ;; happens inside (thunk ...) and generated function bodies
(for-each (lambda (vi) (vinfo:set-asgn! vi #t))
(list-tail (car (lam:vinfo e)) (length (lam:args e))))
Expand Down Expand Up @@ -4501,6 +4511,7 @@ f(x) = yt(x)
((struct_type) "\"struct\" expression")
((method) "method definition")
((set_binding_type!) (string "type declaration for global \"" (deparse (cadr e)) "\""))
((worldinc) "World age increment")
(else (string "\"" h "\" expression"))))
(if (not (null? (cadr lam)))
(error (string (head-to-text (car e)) " not at top level"))))
Expand Down Expand Up @@ -4952,7 +4963,12 @@ f(x) = yt(x)
(else (emit temp)))))

;; top level expressions
((thunk module)
((thunk)
(check-top-level e)
(emit e)
(if tail (emit-return tail '(null)))
'(null))
((module)
(check-top-level e)
(emit e)
(if tail (emit-return tail '(null)))
Expand All @@ -4968,8 +4984,22 @@ f(x) = yt(x)
(if tail (emit-return tail val))
val))

((worldinc-if-toplevel)
(if (null? (cadr lam))
(emit `(worldinc)))
'(null))

((import using)
(check-top-level e)
(emit e)
(emit `(worldinc))
(let ((have-ret? (and (pair? code) (pair? (car code)) (eq? (caar code) 'return))))
(if (and tail (not have-ret?))
(emit-return tail '(null))))
'(null))

;; other top level expressions
((import using export public)
((export public worldinc)
(check-top-level e)
(emit e)
(let ((have-ret? (and (pair? code) (pair? (car code)) (eq? (caar code) 'return))))
Expand Down
1 change: 1 addition & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,7 @@ extern JL_DLLEXPORT jl_sym_t *jl_release_sym;
extern JL_DLLEXPORT jl_sym_t *jl_acquire_release_sym;
extern JL_DLLEXPORT jl_sym_t *jl_sequentially_consistent_sym;
extern JL_DLLEXPORT jl_sym_t *jl_uninferred_sym;
extern JL_DLLEXPORT jl_sym_t *jl_worldinc_sym;

JL_DLLEXPORT enum jl_memory_order jl_get_atomic_order(jl_sym_t *order, char loading, char storing);
JL_DLLEXPORT enum jl_memory_order jl_get_atomic_order_checked(jl_sym_t *order, char loading, char storing);
Expand Down
21 changes: 17 additions & 4 deletions src/toplevel.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,7 @@ static jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex

for (int i = 0; i < jl_array_nrows(exprs); i++) {
// process toplevel form
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
form = jl_expand_stmt_with_loc(jl_array_ptr_ref(exprs, i), newm, filename, lineno);
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
(void)jl_toplevel_eval_flex(newm, form, 1, 1, &filename, &lineno);
}
ct->world_age = last_age;
Expand Down Expand Up @@ -607,7 +605,8 @@ int jl_is_toplevel_only_expr(jl_value_t *e) JL_NOTSAFEPOINT
((jl_expr_t*)e)->head == jl_const_sym ||
((jl_expr_t*)e)->head == jl_toplevel_sym ||
((jl_expr_t*)e)->head == jl_error_sym ||
((jl_expr_t*)e)->head == jl_incomplete_sym);
((jl_expr_t*)e)->head == jl_incomplete_sym ||
((jl_expr_t*)e)->head == jl_worldinc_sym);
}

int jl_needs_lowering(jl_value_t *e) JL_NOTSAFEPOINT
Expand Down Expand Up @@ -864,6 +863,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val

if (head == jl_module_sym) {
jl_value_t *val = jl_eval_module_expr(m, ex);
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
JL_GC_POP();
return val;
}
Expand Down Expand Up @@ -919,6 +919,9 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
jl_eval_errorf(m, *toplevel_filename, *toplevel_lineno,
"syntax: malformed \"using\" statement");
}
if (!expanded) {
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
}
JL_GC_POP();
return jl_nothing;
}
Expand Down Expand Up @@ -967,6 +970,12 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val
jl_eval_errorf(m, *toplevel_filename, *toplevel_lineno,
"syntax: malformed \"import\" statement");
}
if (!expanded) {
// To avoid having to roundtrip every `using` expression through
// lowering, just to add the world-age increment effect, do it
// manually here.
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
}
JL_GC_POP();
return jl_nothing;
}
Expand Down Expand Up @@ -1111,8 +1120,11 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
jl_value_t *v = NULL;
int last_lineno = jl_lineno;
const char *last_filename = jl_filename;
jl_task_t *ct = jl_current_task;
jl_lineno = 1;
jl_filename = "none";
size_t last_age = ct->world_age;
ct->world_age = jl_atomic_load_relaxed(&jl_world_counter);
JL_TRY {
v = jl_toplevel_eval(m, ex);
}
Expand All @@ -1123,6 +1135,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_in(jl_module_t *m, jl_value_t *ex)
}
jl_lineno = last_lineno;
jl_filename = last_filename;
ct->world_age = last_age;
assert(v);
return v;
}
Expand Down Expand Up @@ -1170,6 +1183,7 @@ static jl_value_t *jl_parse_eval_all(jl_module_t *module, jl_value_t *text,
int last_lineno = jl_lineno;
const char *last_filename = jl_filename;
size_t last_age = ct->world_age;
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
int lineno = 0;
jl_lineno = 0;
const char *filename_str = jl_string_data(filename);
Expand All @@ -1187,7 +1201,6 @@ static jl_value_t *jl_parse_eval_all(jl_module_t *module, jl_value_t *text,
}
expression = jl_expand_with_loc_warn(expression, module,
jl_string_data(filename), lineno);
ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
result = jl_toplevel_eval_flex(module, expression, 1, 1, &filename_str, &lineno);
}
}
Expand Down
2 changes: 1 addition & 1 deletion stdlib/Logging/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ end
AboveMaxLevel === Logging.AboveMaxLevel
end
""")
@test m.run()
@test invokelatest(m.run)
end

@testset "custom log macro" begin
Expand Down
1 change: 1 addition & 0 deletions stdlib/REPL/src/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ function eval_user_input(@nospecialize(ast), backend::REPLBackend, mod::Module)
for xf in backend.ast_transforms
ast = Base.invokelatest(xf, ast)
end
toplevel_eval_with_hooks(mod, Expr(:worldinc))
value = toplevel_eval_with_hooks(mod, ast)
backend.in_eval = false
setglobal!(Base.MainInclude, :ans, value)
Expand Down
Loading

0 comments on commit d085375

Please sign in to comment.