Make world-age increments explicit (#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
#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 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 #3 above. I think option #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.

Compiler/src/inferencestate.jl

@@ -209,6 +209,11 @@ to enable flow-sensitive analysis.
 """
 const VarTable = Vector{VarState}
 
+struct StatementState
+    vtypes::Union{VarTable,Nothing}
+    saw_latestworld::Bool
+end
+
 const CACHE_MODE_NULL     = 0x00      # not cached, optimization optional
 const CACHE_MODE_GLOBAL   = 0x01 << 0 # cached globally, optimization required
 const CACHE_MODE_LOCAL    = 0x01 << 1 # cached locally, optimization required
@@ -260,6 +265,7 @@ mutable struct InferenceState
     ssavalue_uses::Vector{BitSet} # ssavalue sparsity and restart info
     # TODO: Could keep this sparsely by doing structural liveness analysis ahead of time.
     bb_vartables::Vector{Union{Nothing,VarTable}} # nothing if not analyzed yet
+    bb_saw_latestworld::Vector{Bool}
     ssavaluetypes::Vector{Any}
     edges::Vector{Any}
     stmt_info::Vector{CallInfo}
@@ -320,6 +326,7 @@ mutable struct InferenceState
 
         nslots = length(src.slotflags)
         slottypes = Vector{Any}(undef, nslots)
+        bb_saw_latestworld = Bool[false for i = 1:length(cfg.blocks)]
         bb_vartables = Union{Nothing,VarTable}[ nothing for i = 1:length(cfg.blocks) ]
         bb_vartable1 = bb_vartables[1] = VarTable(undef, nslots)
         argtypes = result.argtypes
@@ -367,7 +374,7 @@ mutable struct InferenceState
 
         this = new(
             mi, WorldWithRange(world, valid_worlds), mod, sptypes, slottypes, src, cfg, spec_info,
-            currbb, currpc, ip, handler_info, ssavalue_uses, bb_vartables, ssavaluetypes, edges, stmt_info,
+            currbb, currpc, ip, handler_info, ssavalue_uses, bb_vartables, bb_saw_latestworld, ssavaluetypes, edges, stmt_info,
             tasks, pclimitations, limitations, cycle_backedges, callstack, parentid, frameid, cycleid,
             result, unreachable, bestguess, exc_bestguess, ipo_effects,
             restrict_abstract_call_sites, cache_mode, insert_coverage,

Compiler/src/ssair/irinterp.jl

@@ -38,19 +38,19 @@ function abstract_eval_invoke_inst(interp::AbstractInterpreter, inst::Instructio
     mi_cache = WorldView(code_cache(interp), world)
     code = get(mi_cache, mi, nothing)
     code === nothing && return Pair{Any,Tuple{Bool,Bool}}(nothing, (false, false))
-    argtypes = collect_argtypes(interp, stmt.args[2:end], nothing, irsv)
+    argtypes = collect_argtypes(interp, stmt.args[2:end], StatementState(nothing, false), irsv)
     argtypes === nothing && return Pair{Any,Tuple{Bool,Bool}}(Bottom, (false, false))
     return concrete_eval_invoke(interp, code, argtypes, irsv)
 end
 
 abstract_eval_ssavalue(s::SSAValue, sv::IRInterpretationState) = abstract_eval_ssavalue(s, sv.ir)
 
 function abstract_eval_phi_stmt(interp::AbstractInterpreter, phi::PhiNode, ::Int, irsv::IRInterpretationState)
-    return abstract_eval_phi(interp, phi, nothing, irsv)
+    return abstract_eval_phi(interp, phi, StatementState(nothing, false), irsv)
 end
 
-function abstract_call(interp::AbstractInterpreter, arginfo::ArgInfo, irsv::IRInterpretationState)
-    si = StmtInfo(true) # TODO better job here?
+function abstract_call(interp::AbstractInterpreter, arginfo::ArgInfo, sstate::StatementState, irsv::IRInterpretationState)
+    si = StmtInfo(true, sstate.saw_latestworld) # TODO better job here?
     call = abstract_call(interp, arginfo, si, irsv)::Future
     Future{Any}(call, interp, irsv) do call, interp, irsv
         irsv.ir.stmts[irsv.curridx][:info] = call.info
@@ -147,7 +147,7 @@ function reprocess_instruction!(interp::AbstractInterpreter, inst::Instruction,
         if (head === :call || head === :foreigncall || head === :new || head === :splatnew ||
             head === :static_parameter || head === :isdefined || head === :boundscheck)
             @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
-            result = abstract_eval_statement_expr(interp, stmt, nothing, irsv)
+            result = abstract_eval_statement_expr(interp, stmt, StatementState(nothing, false), irsv)
             reverse!(irsv.tasks)
             while true
                 if length(irsv.callstack) > irsv.frameid
@@ -302,7 +302,7 @@ populate_def_use_map!(tpdum::TwoPhaseDefUseMap, ir::IRCode) =
 function is_all_const_call(@nospecialize(stmt), interp::AbstractInterpreter, irsv::IRInterpretationState)
     isexpr(stmt, :call) || return false
     @inbounds for i = 2:length(stmt.args)
-        argtype = abstract_eval_value(interp, stmt.args[i], nothing, irsv)
+        argtype = abstract_eval_value(interp, stmt.args[i], StatementState(nothing, false), irsv)
         is_const_argtype(argtype) || return false
     end
     return true

Compiler/src/tfuncs.jl

@@ -1426,7 +1426,7 @@ end
         # as well as compute the info for the method matches
         op = unwrapva(argtypes[op_argi])
         v = unwrapva(argtypes[v_argi])
-        callinfo = abstract_call(interp, ArgInfo(nothing, Any[op, TF, v]), StmtInfo(true), sv, #=max_methods=#1)
+        callinfo = abstract_call(interp, ArgInfo(nothing, Any[op, TF, v]), StmtInfo(true, si.saw_latestworld), sv, #=max_methods=#1)
         TF = Core.Box(TF)
         RT = Core.Box(RT)
         return Future{CallMeta}(callinfo, interp, sv) do callinfo, interp, sv

Compiler/src/types.jl

@@ -41,6 +41,7 @@ struct StmtInfo
     need thus not be computed.
     """
     used::Bool
+    saw_latestworld::Bool
 end
 
 struct SpecInfo

Compiler/src/validation.jl

@@ -39,6 +39,7 @@ const VALID_EXPR_HEADS = IdDict{Symbol,UnitRange{Int}}(
     :using => 1:typemax(Int),
     :export => 1:typemax(Int),
     :public => 1:typemax(Int),
+    :latestworld => 0:0,
 )
 
 # @enum isn't defined yet, otherwise I'd use it for this

base/boot.jl

@@ -259,13 +259,14 @@ 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

base/essentials.jl

@@ -467,10 +467,18 @@ 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)))
+    return Expr(:let, Expr(:(=), :eval_local_result,
+            Expr(:escape, Expr(:call, GlobalRef(Core, :eval), __module__, Expr(:quote, ex)))),
+        Expr(:block,
+            Expr(:var"latestworld-if-toplevel"),
+            :eval_local_result))
 end
 macro eval(mod, ex)
-    return Expr(:escape, Expr(:call, GlobalRef(Core, :eval), mod, Expr(:quote, ex)))
+    return Expr(:let, Expr(:(=), :eval_local_result,
+            Expr(:escape, Expr(:call, GlobalRef(Core, :eval), mod, Expr(:quote, ex)))),
+        Expr(:block,
+            Expr(:var"latestworld-if-toplevel"),
+            :eval_local_result))
 end
 
 # use `@eval` here to directly form `:new` expressions avoid implicit `convert`s

base/sysimg.jl

@@ -39,6 +39,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.latestworld` 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.latestworld` call after `myinclude`.
+
 !!! compat "Julia 1.5"
     Julia 1.5 is required for passing the `mapexpr` argument.
 """

base/tuple.jl

@@ -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
 
 

src/codegen.cpp

@@ -6721,6 +6721,18 @@ static std::pair<Function*, Function*> get_oc_function(jl_codectx_t &ctx, jl_met
     return std::make_pair(F, specF);
 }
 
+static void emit_latestworld(jl_codectx_t &ctx)
+{
+    auto world_age_field = get_tls_world_age_field(ctx);
+    LoadInst *world = ctx.builder.CreateAlignedLoad(ctx.types().T_size,
+        prepare_global_in(jl_Module, jlgetworld_global), ctx.types().alignof_ptr,
+        /*isVolatile*/false);
+    world->setOrdering(AtomicOrdering::Acquire);
+    StoreInst *store_world = ctx.builder.CreateAlignedStore(world, world_age_field,
+        ctx.types().alignof_ptr, /*isVolatile*/false);
+    (void)store_world;
+}
+
 // `expr` is not clobbered in JL_TRY
 JL_GCC_IGNORE_START("-Wclobbered")
 static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_0based)
@@ -7141,6 +7153,10 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
             ctx.builder.CreateCall(prepare_call(gc_preserve_end_func), {token.V});
         return jl_cgval_t((jl_value_t*)jl_nothing_type);
     }
+    else if (head == jl_latestworld_sym && !jl_is_method(ctx.linfo->def.method)) {
+        emit_latestworld(ctx);
+        return jl_cgval_t((jl_value_t*)jl_nothing_type);
+    }
     else {
         if (jl_is_toplevel_only_expr(expr) &&
             !jl_is_method(ctx.linfo->def.method)) {
@@ -9568,7 +9584,9 @@ static jl_llvm_functions_t
             }
 
             mallocVisitStmt(sync_bytes, have_dbg_update);
-            if (toplevel || ctx.is_opaque_closure)
+            // N.B.: For toplevel thunks, we expect world age restore to be handled
+            // by the interpreter which invokes us.
+            if (ctx.is_opaque_closure)
                 ctx.builder.CreateStore(last_age, world_age_field);
             assert(type_is_ghost(retty) || returninfo.cc == jl_returninfo_t::SRet ||
                 retval->getType() == ctx.f->getReturnType());
@@ -9933,17 +9951,6 @@ static jl_llvm_functions_t
                         I.setDebugLoc(topdebugloc);
                     }
                 }
-                if (toplevel && !ctx.is_opaque_closure && !in_prologue) {
-                    // we're at toplevel; insert an atomic barrier between every instruction
-                    // TODO: inference is invalid if this has any effect (which it often does)
-                    LoadInst *world = new LoadInst(ctx.types().T_size,
-                        prepare_global_in(jl_Module, jlgetworld_global), Twine(),
-                        /*isVolatile*/false, ctx.types().alignof_ptr, /*insertBefore*/&I);
-                    world->setOrdering(AtomicOrdering::Acquire);
-                    StoreInst *store_world = new StoreInst(world, world_age_field,
-                        /*isVolatile*/false, ctx.types().alignof_ptr, /*insertBefore*/&I);
-                    (void)store_world;
-                }
             }
             if (&I == &prologue_end)
                 in_prologue = false;

src/interpreter.c

@@ -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;

src/jlfrontend.scm

@@ -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))))))
 

src/julia-syntax.scm

@@ -1038,6 +1038,7 @@
                                '())))))
          (call (core _typebody!) ,name (call (core svec) ,@(insert-struct-shim field-types name)))
          (const (globalref (thismodule) ,name) ,name)
+         (latestworld)
          (null)))
        ;; "inner" constructors
        (scope-block
@@ -1087,6 +1088,7 @@
                (call (core _equiv_typedef) (globalref (thismodule) ,name) ,name))
            (null)
            (const (globalref (thismodule) ,name) ,name))
+       (latestworld)
        (null))))))
 
 (define (primitive-type-def-expr n name params super)
@@ -1107,6 +1109,7 @@
                (call (core _equiv_typedef) (globalref (thismodule) ,name) ,name))
            (null)
            (const (globalref (thismodule) ,name) ,name))
+       (latestworld)
        (null))))))
 
 ;; take apart a type signature, e.g. T{X} <: S{Y}
@@ -2744,6 +2747,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)) (latestworld-if-toplevel) ,r)))
                  (else
                   (map expand-forms e))))
          (map expand-forms e)))
@@ -4125,15 +4131,17 @@ f(x) = yt(x)
                                      `(lambda ,(cadr lam2)
                                         (,(clear-capture-bits (car vis))
                                          ,@(cdr vis))
-                                        ,body)))))
+                                        ,body)))
+                          (latestworld)))
                        (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))
+                                   (latestworld))
                             ,@top-stmts))))
 
                  ;; local case - lift to a new type at top level
@@ -4272,15 +4280,17 @@ f(x) = yt(x)
                        `(toplevel-butfirst
                          (null)
                          ,@sp-inits
-                         ,@mk-method)
+                         ,@mk-method
+                         (latestworld))
                        (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
+                           (latestworld)))))))))
           ((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))))
@@ -4513,7 +4523,7 @@ f(x) = yt(x)
           ((struct_type)       "\"struct\" expression")
           ((method)            "method definition")
           ((set_binding_type!) (string "type declaration for global \"" (deparse (cadr e)) "\""))
-          ((latestworld)          "World age increment")
+          ((latestworld)       "World age increment")
           (else                (string "\"" h "\" expression"))))
       (if (not (null? (cadr lam)))
           (error (string (head-to-text (car e)) " not at top level"))))
@@ -4965,7 +4975,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)))
@@ -4989,7 +5004,9 @@ f(x) = yt(x)
             ;; other top level expressions
             ((import using export public latestworld)
              (check-top-level e)
-             (emit e)
+             (if (not (eq? (car e) 'latestworld))
+              (emit e))
+             (emit `(latestworld))
              (let ((have-ret? (and (pair? code) (pair? (car code)) (eq? (caar code) 'return))))
                (if (and tail (not have-ret?))
                    (emit-return tail '(null))))