bpart: Also partition ->deprecated (#57449)

This repeats the exercise in #57405. This is required for correctness,
because the ->deprecated flag also affects `using` resolution (since it
makes the tagged binding weaker for `using` purposes). That said, in
general our `->deprecated` semantics have been somewhat underspecified
with lots of `XXX` comments in the surrounding code. This tries to
define the semantics to give a depwarn on *access* (read or write) when:

1. Either the binding itself is deprecated; or
2. The implicit imports pass through a deprecated binding.

However, we do not give depwarns on access to bindings that were
explicitly imported (although we do give those warnings on the import) -
the reasoning being that it's the import that needs to be adjusted not
the access.

Additionally, this PR moves into the direction of making the depwarn a
semantic part of the global access, by adjusting codegen and inference
appropriately.

Author: Keno

Compiler/src/Compiler.jl

@@ -50,6 +50,7 @@ using Core: ABIOverride, Builtin, CodeInstance, IntrinsicFunction, MethodInstanc
 using Base
 using Base: @_foldable_meta, @_gc_preserve_begin, @_gc_preserve_end, @nospecializeinfer,
     BINDING_KIND_GLOBAL, BINDING_KIND_UNDEF_CONST, BINDING_KIND_BACKDATED_CONST, BINDING_KIND_DECLARED,
+    BINDING_FLAG_DEPWARN,
     Base, BitVector, Bottom, Callable, DataTypeFieldDesc,
     EffectsOverride, Filter, Generator, IteratorSize, JLOptions, NUM_EFFECTS_OVERRIDES,
     OneTo, Ordering, RefValue, SizeUnknown, _NAMEDTUPLE_NAME,

Compiler/src/abstractinterpretation.jl

@@ -2375,7 +2375,7 @@ function abstract_throw_methoderror(interp::AbstractInterpreter, argtypes::Vecto
     return Future(CallMeta(Union{}, exct, EFFECTS_THROWS, NoCallInfo()))
 end
 
-const generic_getglobal_effects = Effects(EFFECTS_THROWS, consistent=ALWAYS_FALSE, inaccessiblememonly=ALWAYS_FALSE)
+const generic_getglobal_effects = Effects(EFFECTS_THROWS, effect_free=ALWAYS_FALSE, consistent=ALWAYS_FALSE, inaccessiblememonly=ALWAYS_FALSE) #= effect_free for depwarn =#
 const generic_getglobal_exct = Union{ArgumentError, TypeError, ConcurrencyViolationError, UndefVarError}
 function abstract_eval_getglobal(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, @nospecialize(M), @nospecialize(s))
     ⊑ = partialorder(typeinf_lattice(interp))
@@ -3503,32 +3503,36 @@ end
 
 function abstract_eval_partition_load(interp::AbstractInterpreter, partition::Core.BindingPartition)
     kind = binding_kind(partition)
+    isdepwarn = (partition.kind & BINDING_FLAG_DEPWARN) != 0
+    local_getglobal_effects = Effects(generic_getglobal_effects, effect_free=isdepwarn ? ALWAYS_FALSE : ALWAYS_TRUE)
     if is_some_guard(kind) || kind == BINDING_KIND_UNDEF_CONST
         if InferenceParams(interp).assume_bindings_static
             return RTEffects(Union{}, UndefVarError, EFFECTS_THROWS)
         else
             # We do not currently assume an invalidation for guard -> defined transitions
             # return RTEffects(Union{}, UndefVarError, EFFECTS_THROWS)
-            return RTEffects(Any, UndefVarError, generic_getglobal_effects)
+            return RTEffects(Any, UndefVarError, local_getglobal_effects)
         end
     end
 
     if is_defined_const_binding(kind)
         if kind == BINDING_KIND_BACKDATED_CONST
             # Infer this as guard. We do not want a later const definition to retroactively improve
             # inference results in an earlier world.
-            return RTEffects(Any, UndefVarError, generic_getglobal_effects)
+            return RTEffects(Any, UndefVarError, local_getglobal_effects)
         end
         rt = Const(partition_restriction(partition))
-        return RTEffects(rt, Union{}, Effects(EFFECTS_TOTAL, inaccessiblememonly=is_mutation_free_argtype(rt) ? ALWAYS_TRUE : ALWAYS_FALSE))
+        return RTEffects(rt, Union{}, Effects(EFFECTS_TOTAL,
+            inaccessiblememonly=is_mutation_free_argtype(rt) ? ALWAYS_TRUE : ALWAYS_FALSE,
+            effect_free=isdepwarn ? ALWAYS_FALSE : ALWAYS_TRUE))
     end
 
     if kind == BINDING_KIND_DECLARED
         rt = Any
     else
         rt = partition_restriction(partition)
     end
-    return RTEffects(rt, UndefVarError, generic_getglobal_effects)
+    return RTEffects(rt, UndefVarError, local_getglobal_effects)
 end
 
 function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, saw_latestworld::Bool, sv::AbsIntState)

Compiler/test/irpasses.jl

@@ -1179,7 +1179,10 @@ let ci = code_typed(foo_cfg_empty, Tuple{Bool}, optimize=true)[1][1]
 end
 
 @test Compiler.is_effect_free(Base.infer_effects(getfield, (Complex{Int}, Symbol)))
-@test Compiler.is_effect_free(Base.infer_effects(getglobal, (Module, Symbol)))
+
+# We consider a potential deprecatio warning an effect, so for completely unkown getglobal,
+# we taint the effect_free bit.
+@test !Compiler.is_effect_free(Base.infer_effects(getglobal, (Module, Symbol)))
 
 # Test that UseRefIterator gets SROA'd inside of new_to_regular (#44557)
 # expression and new_to_regular offset are arbitrary here, we just want to see the UseRefIterator erased

base/runtime_internals.jl

@@ -210,6 +210,11 @@ const BINDING_KIND_UNDEF_CONST  = 0x9
 const BINDING_KIND_BACKDATED_CONST = 0xa
 
 const BINDING_FLAG_EXPORTED     = 0x10
+const BINDING_FLAG_DEPRECATED   = 0x20
+const BINDING_FLAG_DEPWARN      = 0x40
+
+const BINDING_KIND_MASK         = 0x0f
+const BINDING_FLAG_MASK         = 0xf0
 
 is_defined_const_binding(kind::UInt8) = (kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_BACKDATED_CONST)
 is_some_const_binding(kind::UInt8) = (is_defined_const_binding(kind) || kind == BINDING_KIND_UNDEF_CONST)

base/show.jl

@@ -3377,8 +3377,21 @@ function print_partition(io::IO, partition::Core.BindingPartition)
     else
         print(io, max_world)
     end
-    if (partition.kind & BINDING_FLAG_EXPORTED) != 0
-        print(io, " [exported]")
+    if (partition.kind & BINDING_FLAG_MASK) != 0
+        first = false
+        print(io, " [")
+        if (partition.kind & BINDING_FLAG_EXPORTED) != 0
+            print(io, "exported")
+        end
+        if (partition.kind & BINDING_FLAG_DEPRECATED) != 0
+            first ? (first = false) : print(io, ",")
+            print(io, "deprecated")
+        end
+        if (partition.kind & BINDING_FLAG_DEPWARN) != 0
+            first ? (first = false) : print(io, ",")
+            print(io, "depwarn")
+        end
+        print(io, "]")
     end
     print(io, " - ")
     kind = binding_kind(partition)

src/codegen.cpp

@@ -908,13 +908,12 @@ static const auto jldeclareglobal_func = new JuliaFunction<>{
             {T_pjlvalue, T_pjlvalue, T_prjlvalue, getInt32Ty(C)}, false); },
     nullptr,
 };
-static const auto jlgetbindingorerror_func = new JuliaFunction<>{
-    XSTR(jl_get_binding_or_error),
+static const auto jldepcheck_func = new JuliaFunction<>{
+    XSTR(jl_binding_deprecation_check),
     [](LLVMContext &C) {
         auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
-        return FunctionType::get(T_pjlvalue,
-                {T_pjlvalue, T_pjlvalue}, false);
-    },
+        return FunctionType::get(getVoidTy(C),
+            {T_pjlvalue}, false); },
     nullptr,
 };
 static const auto jlcheckbpwritable_func = new JuliaFunction<>{
@@ -2894,20 +2893,6 @@ static void mallocVisitLine(jl_codectx_t &ctx, StringRef filename, int line, Val
 
 // --- constant determination ---
 
-static void show_source_loc(jl_codectx_t &ctx, JL_STREAM *out)
-{
-    jl_printf(out, "in %s at %s", ctx.name, ctx.file.str().c_str());
-}
-
-static void cg_bdw(jl_codectx_t &ctx, jl_sym_t *var, jl_binding_t *b)
-{
-    jl_binding_deprecation_warning(ctx.module, var, b);
-    if (b->deprecated == 1 && jl_options.depwarn) {
-        show_source_loc(ctx, JL_STDERR);
-        jl_printf(JL_STDERR, "\n");
-    }
-}
-
 static jl_value_t *static_apply_type(jl_codectx_t &ctx, ArrayRef<jl_cgval_t> args, size_t nargs)
 {
     assert(nargs > 1);
@@ -2932,6 +2917,12 @@ static jl_value_t *static_apply_type(jl_codectx_t &ctx, ArrayRef<jl_cgval_t> arg
     return result;
 }
 
+static void emit_depwarn_check(jl_codectx_t &ctx, jl_binding_t *b)
+{
+    Value *bp = julia_binding_gv(ctx, b);
+    ctx.builder.CreateCall(prepare_call(jldepcheck_func), { bp });
+}
+
 // try to statically evaluate, NULL if not possible. note that this may allocate, and as
 // such the resulting value should not be embedded directly in the generated code.
 static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex)
@@ -2940,9 +2931,13 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex)
         jl_sym_t *sym = (jl_sym_t*)ex;
         jl_binding_t *bnd = jl_get_module_binding(ctx.module, sym, 0);
         jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world);
-        jl_walk_binding_inplace_all(&bnd, &bpart, ctx.min_world, ctx.max_world);
-        if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart)))
+        int possibly_deprecated = 0;
+        jl_walk_binding_inplace_all(&bnd, &bpart, &possibly_deprecated, ctx.min_world, ctx.max_world);
+        if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart))) {
+            if (possibly_deprecated)
+                emit_depwarn_check(ctx, bnd);
             return bpart->restriction;
+        }
         return NULL;
     }
     if (jl_is_slotnumber(ex) || jl_is_argument(ex))
@@ -2965,13 +2960,14 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex)
         s = jl_globalref_name(ex);
         jl_binding_t *bnd = jl_get_module_binding(jl_globalref_mod(ex), s, 0);
         jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world);
-        jl_walk_binding_inplace_all(&bnd, &bpart, ctx.min_world, ctx.max_world);
+        int possibly_deprecated = 0;
+        jl_walk_binding_inplace_all(&bnd, &bpart, &possibly_deprecated, ctx.min_world, ctx.max_world);
         jl_value_t *v = NULL;
         if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart)))
             v = bpart->restriction;
         if (v) {
-            if (bnd->deprecated)
-                cg_bdw(ctx, s, bnd);
+            if (possibly_deprecated)
+                emit_depwarn_check(ctx, bnd);
             return v;
         }
         return NULL;
@@ -2992,13 +2988,14 @@ static jl_value_t *static_eval(jl_codectx_t &ctx, jl_value_t *ex)
                     if (s && jl_is_symbol(s)) {
                         jl_binding_t *bnd = jl_get_module_binding(m, s, 0);
                         jl_binding_partition_t *bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world);
-                        jl_walk_binding_inplace_all(&bnd, &bpart, ctx.min_world, ctx.max_world);
+                        int possibly_deprecated = 0;
+                        jl_walk_binding_inplace_all(&bnd, &bpart, &possibly_deprecated, ctx.min_world, ctx.max_world);
                         jl_value_t *v = NULL;
                         if (bpart && jl_bkind_is_some_constant(jl_binding_kind(bpart)))
                             v = bpart->restriction;
                         if (v) {
-                            if (bnd->deprecated)
-                                cg_bdw(ctx, s, bnd);
+                            if (possibly_deprecated)
+                                emit_depwarn_check(ctx, bnd);
                             return v;
                         }
                     }
@@ -3244,48 +3241,47 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *
     if (!bpart) {
         return emit_globalref_runtime(ctx, bnd, mod, name);
     }
-    // bpart was updated in place - this will change with full partition
-    if (jl_bkind_is_some_guard(jl_binding_kind(bpart))) {
-        // Redo the lookup at runtime
-        return emit_globalref_runtime(ctx, bnd, mod, name);
-    } else {
-        while (true) {
-            if (!bpart)
-                break;
-            if (!jl_bkind_is_some_import(jl_binding_kind(bpart)))
-                break;
-            if (bnd->deprecated) {
-                cg_bdw(ctx, name, bnd);
-            }
-            bnd = (jl_binding_t*)bpart->restriction;
-            bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world);
-            if (!bpart)
-                break;
-        }
-        if (bpart) {
-            enum jl_partition_kind kind = jl_binding_kind(bpart);
-            if (jl_bkind_is_some_constant(kind) && kind != BINDING_KIND_BACKDATED_CONST) {
-                jl_value_t *constval = bpart->restriction;
-                if (!constval) {
-                    undef_var_error_ifnot(ctx, ConstantInt::get(getInt1Ty(ctx.builder.getContext()), 0), name, (jl_value_t*)mod);
-                    return jl_cgval_t();
-                }
-                return mark_julia_const(ctx, constval);
+    int possibly_deprecated = 0;
+    int saw_explicit = 0;
+    while (bpart) {
+        if (!saw_explicit && (bpart->kind & BINDING_FLAG_DEPWARN))
+            possibly_deprecated = 1;
+        enum jl_partition_kind kind = jl_binding_kind(bpart);
+        if (!jl_bkind_is_some_import(kind))
+            break;
+        if (kind != BINDING_KIND_IMPLICIT)
+            saw_explicit = 1;
+        bnd = (jl_binding_t*)bpart->restriction;
+        bpart = jl_get_binding_partition_all(bnd, ctx.min_world, ctx.max_world);
+    }
+    Value *bp = NULL;
+    if (bpart) {
+        enum jl_partition_kind kind = jl_binding_kind(bpart);
+        if (jl_bkind_is_some_constant(kind) && kind != BINDING_KIND_BACKDATED_CONST) {
+            if (possibly_deprecated) {
+                bp = julia_binding_gv(ctx, bnd);
+                ctx.builder.CreateCall(prepare_call(jldepcheck_func), { bp });
+            }
+            jl_value_t *constval = bpart->restriction;
+            if (!constval) {
+                undef_var_error_ifnot(ctx, ConstantInt::get(getInt1Ty(ctx.builder.getContext()), 0), name, (jl_value_t*)mod);
+                return jl_cgval_t();
             }
+            return mark_julia_const(ctx, constval);
         }
     }
     if (!bpart || jl_binding_kind(bpart) != BINDING_KIND_GLOBAL) {
         return emit_globalref_runtime(ctx, bnd, mod, name);
     }
-    Value *bp = julia_binding_gv(ctx, bnd);
-    if (bnd->deprecated) {
-        cg_bdw(ctx, name, bnd);
+    bp = julia_binding_gv(ctx, bnd);
+    if (possibly_deprecated) {
+        ctx.builder.CreateCall(prepare_call(jldepcheck_func), { bp });
     }
     jl_value_t *ty = bpart->restriction;
-    bp = julia_binding_pvalue(ctx, bp);
+    Value *bpval = julia_binding_pvalue(ctx, bp);
     if (ty == nullptr)
         ty = (jl_value_t*)jl_any_type;
-    return update_julia_type(ctx, emit_checked_var(ctx, bp, name, (jl_value_t*)mod, false, ctx.tbaa().tbaa_binding), ty);
+    return update_julia_type(ctx, emit_checked_var(ctx, bpval, name, (jl_value_t*)mod, false, ctx.tbaa().tbaa_binding), ty);
 }
 
 static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *sym, jl_cgval_t rval, const jl_cgval_t &cmp,
@@ -3298,6 +3294,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s
     Value *bp = julia_binding_gv(ctx, bnd);
     if (bpart) {
         if (jl_binding_kind(bpart) == BINDING_KIND_GLOBAL) {
+            int possibly_deprecated = bpart->kind & BINDING_FLAG_DEPWARN;
             jl_value_t *ty = bpart->restriction;
             if (ty != nullptr) {
                 const std::string fname = issetglobal ? "setglobal!" : isreplaceglobal ? "replaceglobal!" : isswapglobal ? "swapglobal!" : ismodifyglobal ? "modifyglobal!" : "setglobalonce!";
@@ -3310,6 +3307,9 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s
                 }
                 bool isboxed = true;
                 bool maybe_null = jl_atomic_load_relaxed(&bnd->value) == NULL;
+                if (possibly_deprecated) {
+                    ctx.builder.CreateCall(prepare_call(jldepcheck_func), { bp });
+                }
                 return typed_store(ctx,
                                 julia_binding_pvalue(ctx, bp),
                                 rval, cmp, ty,
@@ -9913,7 +9913,6 @@ static void init_jit_functions(void)
     add_named_global(memcmp_func, &memcmp);
     add_named_global(jltypeerror_func, &jl_type_error);
     add_named_global(jlcheckassign_func, &jl_checked_assignment);
-    add_named_global(jlgetbindingorerror_func, &jl_get_binding_or_error);
     add_named_global(jlcheckbpwritable_func, &jl_check_binding_currently_writable);
     add_named_global(jlboundp_func, &jl_boundp);
     for (auto it : builtin_func_map())

src/jl_exported_funcs.inc

@@ -194,7 +194,6 @@
     XX(jl_get_backtrace) \
     XX(jl_get_binding) \
     XX(jl_get_binding_for_method_def) \
-    XX(jl_get_binding_or_error) \
     XX(jl_get_binding_wr) \
     XX(jl_check_binding_currently_writable) \
     XX(jl_get_cpu_name) \

src/julia.h

@@ -708,8 +708,21 @@ enum jl_partition_kind {
     BINDING_KIND_IMPLICIT_RECOMPUTE = 0xb
 };
 
-// These are flags that get anded into the above
+static const uint8_t BINDING_KIND_MASK = 0x0f;
+static const uint8_t BINDING_FLAG_MASK = 0xf0;
+
+//// These are flags that get anded into the above
+//
+// _EXPORTED: This binding partition is exported. In the world ranges covered by this partitions,
+// other modules that `using` this module, may implicit import this binding.
 static const uint8_t BINDING_FLAG_EXPORTED       = 0x10;
+// _DEPRECATED: This binding partition is deprecated. It is considered weak for the purposes of
+// implicit import resolution.
+static const uint8_t BINDING_FLAG_DEPRECATED     = 0x20;
+// _DEPWARN: This binding partition will print a deprecation warning on access. Note that _DEPWARN
+// implies _DEPRECATED. However, the reverse is not true. Such bindings are usually used for functions,
+// where calling the function itself will provide a (better) deprecation warning/error.
+static const uint8_t BINDING_FLAG_DEPWARN        = 0x40;
 
 typedef struct __attribute__((aligned(8))) _jl_binding_partition_t {
     JL_DATA_TYPE
@@ -2041,7 +2054,6 @@ JL_DLLEXPORT jl_value_t *jl_get_module_binding_or_nothing(jl_module_t *m, jl_sym
 
 // get binding for reading
 JL_DLLEXPORT jl_binding_t *jl_get_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
-JL_DLLEXPORT jl_binding_t *jl_get_binding_or_error(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var);
 // get binding for assignment