bpart: Redesign representation of implicit imports

# Current Design
When we explicitly import a binding (via either `using M: x` or `import`),
the corresponding bpart representation is a direct pointer to the imported
binding. This is necessary, because that is the precise semantic representation
of the import.

However, for implicit imports (those created via `using M` without explicit
symbol name), the situation is a bit different. Here, there is no semantic
content to the particular binding being imported. Worse, the binding is not
necessarily unique. Our current semantics are essentially that we walk the
entire import graph (both explicit and implicit edges) and if all reachable
terminal nodes are either (i) the same binding or (ii) constant bindings with
the same value, the import is allowed. In this, we are supposed to ignore cycles,
although the current implementation has some trouble with this (#57638, #57699).

If the import succeeds, in the current implementation, we then record an arbitrary
implicit import edge as in the BindingPartition. In essence, we are creating a
spanning tree of the import graph (formed from both the implicit and explicit
import edges). However, dynamic algorithms for spanning tree maintenance are
complicated and we didn't implement any. As a result, it is possible for later
edge additions to accidentally introduce cycles (causing #57699).

An additional problem, even if we could keep a consistent spanning tree, is that
the spanning tree is not unique. In practice, this is not supposed to be observable,
but it is still odd to have a non-unique representation for a particular set of
imports. That said, we don't really need the spanning tree. The only place that
actually uses it is `which(::Module, ::Symbol)` which is not performance sensitive
and arguably a bad API for the above reason that the answer is ill-defined.

# This PR
With all these problems, let's just get rid of the spanning tree all together -
as mentioned we don't really need it. Instead, we split the PARTITION_KIND_IMPLICIT
into two, one for each of the two cases (importing a global binding, or
importing a particular constant value). This is actually a more efficient
implementation for the common case of needing to follow a lookup -
we no longer need to follow all the import edges. In exchange, more work
needs to be done during binding replacement to re-scan the entire import
graph. This is probably the right trade-off though, since binding replacement
is already a slow path.

Fixes #57638, #57699

Author: Keno

Compiler/src/Compiler.jl

@@ -35,10 +35,6 @@ else
 
 @eval baremodule Compiler
 
-# Needs to match UUID defined in Project.toml
-ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), Compiler,
-    (0x807dbc54_b67e_4c79, 0x8afb_eafe4df6f2e1))
-
 using Core.Intrinsics, Core.IR
 
 using Core: ABIOverride, Builtin, CodeInstance, IntrinsicFunction, MethodInstance, MethodMatch,
@@ -61,7 +57,7 @@ using Base: @_foldable_meta, @_gc_preserve_begin, @_gc_preserve_end, @nospeciali
     generating_output, get_nospecializeinfer_sig, get_world_counter, has_free_typevars,
     hasgenerator, hasintersect, indexed_iterate, isType, is_file_tracked, is_function_def,
     is_meta_expr, is_meta_expr_head, is_nospecialized, is_nospecializeinfer, is_defined_const_binding,
-    is_some_const_binding, is_some_guard, is_some_imported, is_valid_intrinsic_elptr,
+    is_some_const_binding, is_some_guard, is_some_imported, is_some_explicit_imported, is_some_binding_imported, is_valid_intrinsic_elptr,
     isbitsunion, isconcretedispatch, isdispatchelem, isexpr, isfieldatomic, isidentityfree,
     iskindtype, ismutabletypename, ismutationfree, issingletontype, isvarargtype, isvatuple,
     kwerr, lookup_binding_partition, may_invoke_generator, methods, midpoint, moduleroot,
@@ -76,6 +72,10 @@ import Base: ==, _topmod, append!, convert, copy, copy!, findall, first, get, ge
     getindex, haskey, in, isempty, isready, iterate, iterate, last, length, max_world,
     min_world, popfirst!, push!, resize!, setindex!, size, intersect
 
+# Needs to match UUID defined in Project.toml
+ccall(:jl_set_module_uuid, Cvoid, (Any, NTuple{2, UInt64}), Compiler,
+    (0x807dbc54_b67e_4c79, 0x8afb_eafe4df6f2e1))
+
 const getproperty = Core.getfield
 const setproperty! = Core.setfield!
 const swapproperty! = Core.swapfield!
@@ -131,7 +131,7 @@ something(x::Any, y...) = x
 ############
 
 baremodule BuildSettings
-using Core: ARGS, include
+using Core: ARGS, include, Int, ===
 using ..Compiler: >, getindex, length
 
 global MAX_METHODS::Int = 3
@@ -191,7 +191,7 @@ macro __SOURCE_FILE__()
 end
 
 module IRShow end # relies on string and IO operations defined in Base
-baremodule TrimVerifier end # relies on IRShow, so define this afterwards
+baremodule TrimVerifier using Core end # relies on IRShow, so define this afterwards
 
 if isdefined(Base, :end_base_include)
     # When this module is loaded as the standard library, include these files as usual

Compiler/src/abstractinterpretation.jl

@@ -3287,7 +3287,7 @@ function abstract_eval_isdefinedglobal(interp::AbstractInterpreter, mod::Module,
     if allow_import !== true
         gr = GlobalRef(mod, sym)
         partition = lookup_binding_partition!(interp, gr, sv)
-        if allow_import !== true && is_some_imported(binding_kind(partition))
+        if allow_import !== true && is_some_binding_imported(binding_kind(partition))
             if allow_import === false
                 rt = Const(false)
             else
@@ -3540,7 +3540,7 @@ end
 
 function walk_binding_partition(imported_binding::Core.Binding, partition::Core.BindingPartition, world::UInt)
     valid_worlds = WorldRange(partition.min_world, partition.max_world)
-    while is_some_imported(binding_kind(partition))
+    while is_some_binding_imported(binding_kind(partition))
         imported_binding = partition_restriction(partition)::Core.Binding
         partition = lookup_binding_partition(world, imported_binding)
         valid_worlds = intersect(valid_worlds, WorldRange(partition.min_world, partition.max_world))

Compiler/src/ssair/EscapeAnalysis.jl

@@ -15,6 +15,7 @@ using Base: Base
 # imports
 import Base: ==, copy, getindex, setindex!
 # usings
+using Core
 using Core: Builtin, IntrinsicFunction, SimpleVector, ifelse, sizeof
 using Core.IR
 using Base:       # Base definitions

Compiler/test/codegen.jl

@@ -1007,3 +1007,23 @@ let
    end
    nothing
 end
+
+# Test that turning an implicit import into an explicit one doesn't pessimize codegen
+module TurnedIntoExplicit
+    using Test
+    import ..get_llvm
+
+    module ReExportBitCast
+        export bitcast
+        import Base: bitcast
+    end
+    using .ReExportBitCast
+
+    f(x::UInt) = bitcast(Float64, x)
+
+    @test !occursin("jl_apply_generic", get_llvm(f, Tuple{UInt}))
+
+    import Base: bitcast
+
+    @test !occursin("jl_apply_generic", get_llvm(f, Tuple{UInt}))
+end

base/Base_compiler.jl

@@ -277,18 +277,21 @@ include("operators.jl")
 include("pointer.jl")
 include("refvalue.jl")
 include("cmem.jl")
+
+function nextfloat end
+function prevfloat end
 include("rounding.jl")
 include("float.jl")
 
-include("checked.jl")
-using .Checked
-function cld end
-function fld end
-
 # Lazy strings
 import Core: String
 include("strings/lazy.jl")
 
+function cld end
+function fld end
+include("checked.jl")
+using .Checked
+
 # array structures
 include("indices.jl")
 include("genericmemory.jl")
@@ -373,3 +376,4 @@ Core._setparser!(fl_parse)
 # Further definition of Base will happen in Base.jl if loaded.
 
 end # baremodule Base
+using .Base

base/errorshow.jl

@@ -1159,7 +1159,7 @@ function UndefVarError_hint(io::IO, ex::UndefVarError)
                 "with the module it should come from.")
             elseif kind === Base.PARTITION_KIND_GUARD
                 print(io, "\nSuggestion: check for spelling errors or missing imports.")
-            elseif Base.is_some_imported(kind)
+            elseif Base.is_some_explicit_imported(kind)
                 print(io, "\nSuggestion: this global was defined as `$(Base.partition_restriction(bpart).globalref)` but not assigned a value.")
             end
         elseif scope === :static_parameter

base/invalidation.jl

@@ -151,7 +151,9 @@ function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core
                     latest_bpart = edge.partitions
                     latest_bpart.max_world == typemax(UInt) || continue
                     is_some_imported(binding_kind(latest_bpart)) || continue
-                    partition_restriction(latest_bpart) === b || continue
+                    if is_some_binding_imported(binding_kind(latest_bpart))
+                        partition_restriction(latest_bpart) === b || continue
+                    end
                     invalidate_code_for_globalref!(edge, latest_bpart, latest_bpart, new_max_world)
                 else
                     invalidate_method_for_globalref!(gr, edge::Method, invalidated_bpart, new_max_world)
@@ -171,9 +173,9 @@ function invalidate_code_for_globalref!(b::Core.Binding, invalidated_bpart::Core
                 isdefined(user_binding, :partitions) || continue
                 latest_bpart = user_binding.partitions
                 latest_bpart.max_world == typemax(UInt) || continue
-                binding_kind(latest_bpart) in (PARTITION_KIND_IMPLICIT, PARTITION_KIND_FAILED, PARTITION_KIND_GUARD) || continue
+                is_some_implicit(binding_kind(latest_bpart)) || continue
                 new_bpart = need_to_invalidate_export ?
-                    ccall(:jl_maybe_reresolve_implicit, Any, (Any, Any, Csize_t), user_binding, latest_bpart, new_max_world) :
+                    ccall(:jl_maybe_reresolve_implicit, Any, (Any, Csize_t), user_binding, new_max_world) :
                     latest_bpart
                 if need_to_invalidate_code || new_bpart !== latest_bpart
                     invalidate_code_for_globalref!(convert(Core.Binding, user_binding), latest_bpart, new_bpart, new_max_world)

base/iterators.jl

@@ -17,6 +17,7 @@ using .Base:
     any, _counttuple, eachindex, ntuple, zero, prod, reduce, in, firstindex, lastindex,
     tail, fieldtypes, min, max, minimum, zero, oneunit, promote, promote_shape, LazyString,
     afoldl
+using Core
 using Core: @doc
 
 using .Base:

base/runtime_internals.jl

@@ -197,17 +197,18 @@ function _fieldnames(@nospecialize t)
 end
 
 # N.B.: Needs to be synced with julia.h
-const PARTITION_KIND_CONST        = 0x0
-const PARTITION_KIND_CONST_IMPORT = 0x1
-const PARTITION_KIND_GLOBAL       = 0x2
-const PARTITION_KIND_IMPLICIT     = 0x3
-const PARTITION_KIND_EXPLICIT     = 0x4
-const PARTITION_KIND_IMPORTED     = 0x5
-const PARTITION_KIND_FAILED       = 0x6
-const PARTITION_KIND_DECLARED     = 0x7
-const PARTITION_KIND_GUARD        = 0x8
-const PARTITION_KIND_UNDEF_CONST  = 0x9
-const PARTITION_KIND_BACKDATED_CONST = 0xa
+const PARTITION_KIND_CONST              = 0x0
+const PARTITION_KIND_CONST_IMPORT       = 0x1
+const PARTITION_KIND_GLOBAL             = 0x2
+const PARTITION_KIND_IMPLICIT_GLOBAL    = 0x3
+const PARTITION_KIND_IMPLICIT_CONST     = 0x4
+const PARTITION_KIND_EXPLICIT           = 0x5
+const PARTITION_KIND_IMPORTED           = 0x6
+const PARTITION_KIND_FAILED             = 0x7
+const PARTITION_KIND_DECLARED           = 0x8
+const PARTITION_KIND_GUARD              = 0x9
+const PARTITION_KIND_UNDEF_CONST        = 0xa
+const PARTITION_KIND_BACKDATED_CONST    = 0xb
 
 const PARTITION_FLAG_EXPORTED     = 0x10
 const PARTITION_FLAG_DEPRECATED   = 0x20
@@ -218,9 +219,12 @@ const PARTITION_MASK_FLAG         = 0xf0
 
 const BINDING_FLAG_ANY_IMPLICIT_EDGES = 0x8
 
-is_defined_const_binding(kind::UInt8) = (kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_BACKDATED_CONST)
+is_defined_const_binding(kind::UInt8) = (kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_BACKDATED_CONST)
 is_some_const_binding(kind::UInt8) = (is_defined_const_binding(kind) || kind == PARTITION_KIND_UNDEF_CONST)
-is_some_imported(kind::UInt8) = (kind == PARTITION_KIND_IMPLICIT || kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED)
+is_some_imported(kind::UInt8) = (kind == PARTITION_KIND_IMPLICIT_GLOBAL || kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED)
+is_some_implicit(kind::UInt8) = (kind == PARTITION_KIND_IMPLICIT_GLOBAL || kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_GUARD || kind == PARTITION_KIND_FAILED)
+is_some_explicit_imported(kind::UInt8) = (kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED)
+is_some_binding_imported(kind::UInt8) = is_some_explicit_imported(kind) || kind == PARTITION_KIND_IMPLICIT_GLOBAL
 is_some_guard(kind::UInt8) = (kind == PARTITION_KIND_GUARD || kind == PARTITION_KIND_FAILED || kind == PARTITION_KIND_UNDEF_CONST)
 
 function lookup_binding_partition(world::UInt, b::Core.Binding)

base/show.jl

@@ -1023,14 +1023,20 @@ end
 function isvisible(sym::Symbol, parent::Module, from::Module)
     isdeprecated(parent, sym) && return false
     isdefinedglobal(from, sym) || return false
+    isdefinedglobal(parent, sym) || return false
     parent_binding = convert(Core.Binding, GlobalRef(parent, sym))
     from_binding = convert(Core.Binding, GlobalRef(from, sym))
     while true
         from_binding === parent_binding && return true
         partition = lookup_binding_partition(tls_world_age(), from_binding)
-        is_some_imported(binding_kind(partition)) || break
+        is_some_explicit_imported(binding_kind(partition)) || break
         from_binding = partition_restriction(partition)::Core.Binding
     end
+    parent_partition = lookup_binding_partition(tls_world_age(), parent_binding)
+    from_partition = lookup_binding_partition(tls_world_age(), from_binding)
+    if is_defined_const_binding(binding_kind(parent_partition)) && is_defined_const_binding(binding_kind(from_partition))
+        return parent_partition.restriction === from_partition.restriction
+    end
     return false
 end
 
@@ -3398,7 +3404,7 @@ function print_partition(io::IO, partition::Core.BindingPartition)
     if kind == PARTITION_KIND_BACKDATED_CONST
         print(io, "backdated constant binding to ")
         print(io, partition_restriction(partition))
-    elseif is_defined_const_binding(kind)
+    elseif kind == PARTITION_KIND_CONST
         print(io, "constant binding to ")
         print(io, partition_restriction(partition))
     elseif kind == PARTITION_KIND_UNDEF_CONST
@@ -3409,9 +3415,12 @@ function print_partition(io::IO, partition::Core.BindingPartition)
         print(io, "ambiguous binding - guard entry")
     elseif kind == PARTITION_KIND_DECLARED
         print(io, "weak global binding declared using `global` (implicit type Any)")
-    elseif kind == PARTITION_KIND_IMPLICIT
-        print(io, "implicit `using` from ")
+    elseif kind == PARTITION_KIND_IMPLICIT_GLOBAL
+        print(io, "implicit `using` resolved to global ")
         print(io, partition_restriction(partition).globalref)
+    elseif kind == PARTITION_KIND_IMPLICIT_CONST
+        print(io, "implicit `using` resolved to constant ")
+        print(io, partition_restriction(partition))
     elseif kind == PARTITION_KIND_EXPLICIT
         print(io, "explicit `using` from ")
         print(io, partition_restriction(partition).globalref)
@@ -3434,7 +3443,7 @@ function show(io::IO, ::MIME"text/plain", bnd::Core.Binding)
     print(io, "Binding ")
     print(io, bnd.globalref)
     if !isdefined(bnd, :partitions)
-        print(io, "No partitions")
+        print(io, " - No partitions")
     else
         partition = @atomic bnd.partitions
         while true

base/sysimg.jl

@@ -12,8 +12,6 @@ Core.include(Base, "Base.jl")
 had_compiler && ccall(:jl_init_restored_module, Cvoid, (Any,), Base)
 end
 
-using .Base
-
 # Set up Main module
 using Base.MainInclude # ans, err, and sometimes Out
 

src/jl_exported_funcs.inc

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

src/julia.h

@@ -632,7 +632,8 @@ typedef struct _jl_weakref_t {
 //   These binding kinds depend solely on the set of using'd packages and are not explicitly
 //   declared:
 //
-//      PARTITION_KIND_IMPLICIT
+//      PARTITION_KIND_IMPLICIT_CONST
+//      PARTITION_KIND_IMPLICIT_GLOBAL
 //      PARTITION_KIND_GUARD
 //      PARTITION_KIND_FAILED
 //
@@ -683,37 +684,41 @@ enum jl_partition_kind {
     // `global x::T` to implicitly through a syntactic global assignment.
     //  -> restriction holds the type restriction
     PARTITION_KIND_GLOBAL       = 0x2,
-    // Implicit: The binding was implicitly imported from a `using`'d module.
-    //  ->restriction holds the imported binding
-    PARTITION_KIND_IMPLICIT     = 0x3,
+    // Implicit: The binding was a global, implicitly imported from a `using`'d module.
+    //  ->restriction holds the ultimately imported global binding
+    PARTITION_KIND_IMPLICIT_GLOBAL     = 0x3,
+    // Implicit: The binding was a constant, implicitly imported from a `using`'d module.
+    //  ->restriction holds the ultimately imported constant value
+    PARTITION_KIND_IMPLICIT_CONST     = 0x4,
     // Explicit: The binding was explicitly `using`'d by name
     //  ->restriction holds the imported binding
-    PARTITION_KIND_EXPLICIT     = 0x4,
+    PARTITION_KIND_EXPLICIT     = 0x5,
     // Imported: The binding was explicitly `import`'d by name
     //  ->restriction holds the imported binding
-    PARTITION_KIND_IMPORTED     = 0x5,
+    PARTITION_KIND_IMPORTED     = 0x6,
     // Failed: We attempted to import the binding, but the import was ambiguous
     //  ->restriction is NULL.
-    PARTITION_KIND_FAILED       = 0x6,
+    PARTITION_KIND_FAILED       = 0x7,
     // Declared: The binding was declared using `global` or similar. This acts in most ways like
     // PARTITION_KIND_GLOBAL with an `Any` restriction, except that it may be redefined to a stronger
     // binding like `const` or an explicit import.
     //  ->restriction is NULL.
-    PARTITION_KIND_DECLARED     = 0x7,
+    PARTITION_KIND_DECLARED     = 0x8,
     // Guard: The binding was looked at, but no global or import was resolved at the time
     //  ->restriction is NULL.
-    PARTITION_KIND_GUARD        = 0x8,
+    PARTITION_KIND_GUARD        = 0x9,
     // Undef Constant: This binding partition is a constant declared using `const`, but
     // without a value.
     //  ->restriction is NULL
-    PARTITION_KIND_UNDEF_CONST  = 0x9,
+    PARTITION_KIND_UNDEF_CONST  = 0xa,
     // Backated constant. A constant that was backdated for compatibility. In all other
     // ways equivalent to PARTITION_KIND_CONST, but prints a warning on access
-    PARTITION_KIND_BACKDATED_CONST = 0xa,
+    PARTITION_KIND_BACKDATED_CONST = 0xb,
 
     // This is not a real binding kind, but can be used to ask for a re-resolution
     // of the implicit binding kind
-    PARTITION_KIND_IMPLICIT_RECOMPUTE = 0xb
+    PARTITION_FAKE_KIND_IMPLICIT_RECOMPUTE = 0xc,
+    PARTITION_FAKE_KIND_CYCLE = 0xd
 };
 
 static const uint8_t PARTITION_MASK_KIND = 0x0f;
@@ -745,9 +750,9 @@ typedef struct JL_ALIGNED_ATTR(8) _jl_binding_partition_t {
     /* union {
      *   // For ->kind == PARTITION_KIND_GLOBAL
      *   jl_value_t *type_restriction;
-     *   // For ->kind == PARTITION_KIND_CONST(_IMPORT)
+     *   // For ->kind in (PARTITION_KIND_CONST(_IMPORT), PARTITION_KIND_IMPLICIT_CONST)
      *   jl_value_t *constval;
-     *   // For ->kind in (PARTITION_KIND_IMPLICIT, PARTITION_KIND_EXPLICIT, PARTITION_KIND_IMPORT)
+     *   // For ->kind in (PARTITION_KIND_IMPLICIT_GLOBAL, PARTITION_KIND_EXPLICIT, PARTITION_KIND_IMPORT)
      *   jl_binding_t *imported;
      * } restriction;
      */
@@ -2085,7 +2090,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var);
 // get binding for assignment
 JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module_t *m, jl_sym_t *s);
 JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
-JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, size_t new_world);
+JL_DLLEXPORT jl_value_t *jl_get_existing_strong_gf(jl_binding_t *b JL_PROPAGATES_ROOT, size_t new_world);
 JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var, int allow_import);
 JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var);
 JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var);