From a46c5e0018f0db64cb637b7319e4fa57384c81cb Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 13 Mar 2025 07:22:50 +0000 Subject: [PATCH] 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 --- Compiler/src/Compiler.jl | 14 +- Compiler/src/abstractinterpretation.jl | 4 +- Compiler/src/ssair/EscapeAnalysis.jl | 1 + Compiler/test/codegen.jl | 20 + base/Base_compiler.jl | 14 +- base/errorshow.jl | 2 +- base/invalidation.jl | 8 +- base/iterators.jl | 1 + base/runtime_internals.jl | 30 +- base/show.jl | 19 +- base/sysimg.jl | 2 - src/jl_exported_funcs.inc | 1 - src/julia.h | 35 +- src/julia_internal.h | 29 +- src/method.c | 22 +- src/module.c | 552 +++++++++++++++---------- src/staticdata.c | 6 +- src/toplevel.c | 6 +- test/core.jl | 52 ++- test/reflection.jl | 21 + 20 files changed, 532 insertions(+), 307 deletions(-) diff --git a/Compiler/src/Compiler.jl b/Compiler/src/Compiler.jl index 82af7ed681dd6..298ddd729ddd0 100644 --- a/Compiler/src/Compiler.jl +++ b/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 diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index 7bd7521aba48e..7ceb0094edeac 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/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)) diff --git a/Compiler/src/ssair/EscapeAnalysis.jl b/Compiler/src/ssair/EscapeAnalysis.jl index af8e9b1a4959e..4ce972937700c 100644 --- a/Compiler/src/ssair/EscapeAnalysis.jl +++ b/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 diff --git a/Compiler/test/codegen.jl b/Compiler/test/codegen.jl index 4514852a2c0bc..4e0a1b1f88997 100644 --- a/Compiler/test/codegen.jl +++ b/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 diff --git a/base/Base_compiler.jl b/base/Base_compiler.jl index 640b8226788cb..911659034a145 100644 --- a/base/Base_compiler.jl +++ b/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 diff --git a/base/errorshow.jl b/base/errorshow.jl index e9f8a9a30db74..c53c605cbfb86 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -1161,7 +1161,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 diff --git a/base/invalidation.jl b/base/invalidation.jl index 5b48af0171205..14b88e71b9def 100644 --- a/base/invalidation.jl +++ b/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) diff --git a/base/iterators.jl b/base/iterators.jl index d6367ed8d996e..c7450781c4928 100644 --- a/base/iterators.jl +++ b/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: diff --git a/base/runtime_internals.jl b/base/runtime_internals.jl index 846c765ff64b7..239df932d4bc6 100644 --- a/base/runtime_internals.jl +++ b/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) diff --git a/base/show.jl b/base/show.jl index e91b293a0a0c0..4d9ee1ef25940 100644 --- a/base/show.jl +++ b/base/show.jl @@ -1030,14 +1030,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 @@ -3405,7 +3411,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 @@ -3416,9 +3422,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) @@ -3441,7 +3450,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 diff --git a/base/sysimg.jl b/base/sysimg.jl index 42f54a849f157..c12ddcd71c66f 100644 --- a/base/sysimg.jl +++ b/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 diff --git a/src/jl_exported_funcs.inc b/src/jl_exported_funcs.inc index 94f48757a0745..e4fd664478123 100644 --- a/src/jl_exported_funcs.inc +++ b/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) \ diff --git a/src/julia.h b/src/julia.h index 5255eba13d681..cc1e35e89f5ca 100644 --- a/src/julia.h +++ b/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); diff --git a/src/julia_internal.h b/src/julia_internal.h index 0d5ac60c9255d..58f7ab96a9fcc 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -855,7 +855,6 @@ typedef struct _modstack_t { jl_binding_t *b; struct _modstack_t *prev; } modstack_t; -void jl_check_new_binding_implicit(jl_binding_partition_t *new_bpart, jl_binding_t *b, modstack_t *st, size_t world); #ifndef __clang_gcanalyzer__ // The analyzer doesn't like looking through the arraylist, so just model the @@ -918,6 +917,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked(jl_binding_t *b J jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, enum jl_partition_kind kind, size_t new_world) JL_GLOBALLY_ROOTED; JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b JL_PROPAGATES_ROOT, jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, size_t kind, size_t new_world) JL_GLOBALLY_ROOTED; +JL_DLLEXPORT void jl_update_loaded_bpart(jl_binding_t *b, jl_binding_partition_t *bpart); extern jl_array_t *jl_module_init_order JL_GLOBALLY_ROOTED; extern htable_t jl_current_modules JL_GLOBALLY_ROOTED; extern JL_DLLEXPORT jl_module_t *jl_precompile_toplevel_module JL_GLOBALLY_ROOTED; @@ -937,7 +937,11 @@ jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name JL_DLLEXPORT int jl_is_valid_oc_argtype(jl_tupletype_t *argt, jl_method_t *source); STATIC_INLINE int jl_bkind_is_some_import(enum jl_partition_kind kind) JL_NOTSAFEPOINT { - return kind == PARTITION_KIND_IMPLICIT || kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED; + return kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_IMPLICIT_GLOBAL || kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED; +} + +STATIC_INLINE int jl_bkind_is_some_explicit_import(enum jl_partition_kind kind) JL_NOTSAFEPOINT { + return kind == PARTITION_KIND_EXPLICIT || kind == PARTITION_KIND_IMPORTED; } STATIC_INLINE int jl_bkind_is_some_guard(enum jl_partition_kind kind) JL_NOTSAFEPOINT { @@ -945,15 +949,15 @@ STATIC_INLINE int jl_bkind_is_some_guard(enum jl_partition_kind kind) JL_NOTSAFE } STATIC_INLINE int jl_bkind_is_some_implicit(enum jl_partition_kind kind) JL_NOTSAFEPOINT { - return kind == PARTITION_KIND_IMPLICIT || jl_bkind_is_some_guard(kind); + return kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_IMPLICIT_GLOBAL || jl_bkind_is_some_guard(kind); } STATIC_INLINE int jl_bkind_is_some_constant(enum jl_partition_kind kind) JL_NOTSAFEPOINT { - return kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_UNDEF_CONST || kind == PARTITION_KIND_BACKDATED_CONST; + return kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_UNDEF_CONST || kind == PARTITION_KIND_BACKDATED_CONST; } STATIC_INLINE int jl_bkind_is_defined_constant(enum jl_partition_kind kind) JL_NOTSAFEPOINT { - return kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_BACKDATED_CONST; + return kind == PARTITION_KIND_IMPLICIT_CONST || kind == PARTITION_KIND_CONST || kind == PARTITION_KIND_CONST_IMPORT || kind == PARTITION_KIND_BACKDATED_CONST; } JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world) JL_GLOBALLY_ROOTED; @@ -982,7 +986,8 @@ STATIC_INLINE void jl_walk_binding_inplace_worlds(jl_binding_t **bnd, jl_binding STATIC_INLINE void jl_walk_binding_inplace(jl_binding_t **bnd, jl_binding_partition_t **bpart, size_t world) JL_NOTSAFEPOINT { while (1) { - if (!jl_bkind_is_some_import(jl_binding_kind(*bpart))) + enum jl_partition_kind kind = jl_binding_kind(*bpart); + if (!jl_bkind_is_some_explicit_import(kind) && kind != PARTITION_KIND_IMPLICIT_GLOBAL) return; *bnd = (jl_binding_t*)(*bpart)->restriction; *bpart = jl_get_binding_partition(*bnd, world); @@ -994,14 +999,14 @@ STATIC_INLINE void jl_walk_binding_inplace_depwarn(jl_binding_t **bnd, jl_bindin int passed_explicit = 0; while (1) { enum jl_partition_kind kind = jl_binding_kind(*bpart); - if (!jl_bkind_is_some_import(kind)) { + if (!jl_bkind_is_some_explicit_import(kind) && kind != PARTITION_KIND_IMPLICIT_GLOBAL) { if (!passed_explicit && depwarn) *depwarn |= (*bpart)->kind & PARTITION_FLAG_DEPWARN; return; } if (!passed_explicit && depwarn) *depwarn |= (*bpart)->kind & PARTITION_FLAG_DEPWARN; - if (kind != PARTITION_KIND_IMPLICIT) + if (kind != PARTITION_KIND_IMPLICIT_GLOBAL) passed_explicit = 1; *bnd = (jl_binding_t*)(*bpart)->restriction; *bpart = jl_get_binding_partition(*bnd, world); @@ -1014,14 +1019,14 @@ STATIC_INLINE void jl_walk_binding_inplace_all(jl_binding_t **bnd, jl_binding_pa int passed_explicit = 0; while (*bpart) { enum jl_partition_kind kind = jl_binding_kind(*bpart); - if (!jl_bkind_is_some_import(kind)) { + if (!jl_bkind_is_some_explicit_import(kind) && kind != PARTITION_KIND_IMPLICIT_GLOBAL) { if (!passed_explicit && depwarn) *depwarn |= (*bpart)->kind & PARTITION_FLAG_DEPWARN; return; } if (!passed_explicit && depwarn) *depwarn |= (*bpart)->kind & PARTITION_FLAG_DEPWARN; - if (kind != PARTITION_KIND_IMPLICIT) + if (kind != PARTITION_KIND_IMPLICIT_GLOBAL) passed_explicit = 1; *bnd = (jl_binding_t*)(*bpart)->restriction; *bpart = jl_get_binding_partition_all(*bnd, min_world, max_world); @@ -1038,14 +1043,14 @@ STATIC_INLINE void jl_walk_binding_inplace_worlds(jl_binding_t **bnd, jl_binding if (*max_world > bpart_max_world) *max_world = bpart_max_world; enum jl_partition_kind kind = jl_binding_kind(*bpart); - if (!jl_bkind_is_some_import(kind)) { + if (!jl_bkind_is_some_explicit_import(kind) && kind != PARTITION_KIND_IMPLICIT_GLOBAL) { if (!passed_explicit && depwarn) *depwarn |= (*bpart)->kind & PARTITION_FLAG_DEPWARN; return; } if (!passed_explicit && depwarn) *depwarn |= (*bpart)->kind & PARTITION_FLAG_DEPWARN; - if (kind != PARTITION_KIND_IMPLICIT) + if (kind != PARTITION_KIND_IMPLICIT_GLOBAL) passed_explicit = 1; *bnd = (jl_binding_t*)(*bpart)->restriction; *bpart = jl_get_binding_partition(*bnd, world); diff --git a/src/method.c b/src/method.c index 11609f9d3b61d..353fbdaf0ecff 100644 --- a/src/method.c +++ b/src/method.c @@ -1102,26 +1102,18 @@ JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_module_t *mod, jl_sym_t *name) { JL_LOCK(&world_counter_lock); size_t new_world = jl_atomic_load_relaxed(&jl_world_counter) + 1; - jl_binding_t *b = jl_get_binding_for_method_def(mod, name, new_world); - jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); - jl_value_t *gf = NULL; - enum jl_partition_kind kind = jl_binding_kind(bpart); - if (!jl_bkind_is_some_guard(kind) && kind != PARTITION_KIND_DECLARED && kind != PARTITION_KIND_IMPLICIT) { - jl_walk_binding_inplace(&b, &bpart, new_world); - if (jl_bkind_is_some_constant(jl_binding_kind(bpart))) { - gf = bpart->restriction; - JL_GC_PROMISE_ROOTED(gf); - jl_check_gf(gf, b->globalref->name); - JL_UNLOCK(&world_counter_lock); - return gf; - } - jl_errorf("cannot define function %s; it already has a value", jl_symbol_name(name)); + jl_binding_t *b = jl_get_module_binding(mod, name, 1); + jl_value_t *gf = jl_get_existing_strong_gf(b, new_world); + if (gf) { + jl_check_gf(gf, name); + JL_UNLOCK(&world_counter_lock); + return gf; } gf = (jl_value_t*)jl_new_generic_function(name, mod, new_world); // From this point on (if we didn't error), we're committed to raising the world age, // because we've used it to declare the type name. - jl_atomic_store_release(&jl_world_counter, new_world); jl_declare_constant_val3(b, mod, name, gf, PARTITION_KIND_CONST, new_world); + jl_atomic_store_release(&jl_world_counter, new_world); JL_GC_PROMISE_ROOTED(gf); JL_UNLOCK(&world_counter_lock); return gf; diff --git a/src/module.c b/src/module.c index 0c7db7ab63268..07b79b707f024 100644 --- a/src/module.c +++ b/src/module.c @@ -26,54 +26,146 @@ static jl_binding_partition_t *new_binding_partition(void) return bpart; } +struct implicit_search_gap { + _Atomic(jl_binding_partition_t *) *insert; + jl_binding_partition_t *replace; + jl_value_t *parent; + + size_t min_world; + size_t max_world; + size_t inherited_flags; +}; + +STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition__(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world, struct implicit_search_gap *gap) JL_GLOBALLY_ROOTED +{ + // Iterate through the list of binding partitions, keeping track of where to insert a new one for an implicit + // resolution if necessary. + while (gap->replace && world < gap->replace->min_world) { + gap->insert = &gap->replace->next; + gap->max_world = gap->replace->min_world - 1; + gap->parent = (jl_value_t*)gap->replace; + gap->replace = jl_atomic_load_relaxed(gap->insert); + } + if (gap->replace && world <= jl_atomic_load_relaxed(&gap->replace->max_world)) { + return gap->replace; + } + gap->min_world = gap->replace ? jl_atomic_load_relaxed(&gap->replace->max_world) + 1 : 0; + if (gap->replace) + gap->inherited_flags = gap->replace->kind & PARTITION_MASK_FLAG; + else + gap->inherited_flags = 0; + return NULL; +} -static jl_binding_partition_t *jl_get_binding_partition2(jl_binding_t *b, size_t world, modstack_t *st); +STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_if_present(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world, struct implicit_search_gap *gap) +{ + gap->parent = (jl_value_t*)b; + gap->insert = &b->partitions; + gap->replace = jl_atomic_load_relaxed(gap->insert); + gap->min_world = 0; + gap->max_world = ~(size_t)0; + gap->inherited_flags = 0; + return jl_get_binding_partition__(b, world, gap); +} + +struct implicit_search_resolution { + enum jl_partition_kind ultimate_kind; + jl_value_t *binding_or_const; + size_t min_world; + size_t max_world; + int saw_cycle; + //// Not semantic, but used for reflection. + // If non-null, the unique module from which this binding was imported + jl_module_t *debug_only_import_from; + // If non-null, the unique binding imported. For PARTITION_KIND_IMPLICIT_GLOBAL, always matches binding_or_const. + // Must have trust_cache = 0. + jl_binding_t *debug_only_ultimate_binding; +}; + +static size_t WORLDMAX(size_t a, size_t b) { return a > b ? a : b; } +static size_t WORLDMIN(size_t a, size_t b) { return a > b ? b : a; } + +static void update_implicit_resolution(struct implicit_search_resolution *to_update, struct implicit_search_resolution resolution) +{ + to_update->min_world = WORLDMAX(to_update->min_world, resolution.min_world); + to_update->max_world = WORLDMIN(to_update->max_world, resolution.max_world); + to_update->saw_cycle |= resolution.saw_cycle; + if (resolution.ultimate_kind == PARTITION_FAKE_KIND_CYCLE) { + // Cycles get ignored. This causes the resolution resolution to only be partial, so we can't + // cache it. This gets tracked in saw_cycle; + to_update->saw_cycle = 1; + return; + } + if (resolution.ultimate_kind == PARTITION_KIND_GUARD) { + // Ignore guard imports + return; + } + if (to_update->ultimate_kind == PARTITION_KIND_GUARD) { + assert(resolution.binding_or_const); + to_update->ultimate_kind = resolution.ultimate_kind; + to_update->binding_or_const = resolution.binding_or_const; + to_update->debug_only_import_from = resolution.debug_only_import_from; + to_update->debug_only_ultimate_binding = resolution.debug_only_ultimate_binding; + return; + } + if (resolution.ultimate_kind == to_update->ultimate_kind && + resolution.binding_or_const == to_update->binding_or_const) { + if (resolution.debug_only_import_from != to_update->debug_only_import_from) { + to_update->debug_only_import_from = NULL; + } + if (resolution.debug_only_ultimate_binding != to_update->debug_only_ultimate_binding) { + to_update->debug_only_ultimate_binding = NULL; + } + return; + } + to_update->ultimate_kind = PARTITION_KIND_FAILED; + to_update->binding_or_const = NULL; + to_update->debug_only_import_from = NULL; + to_update->debug_only_ultimate_binding = NULL; +} -static int eq_bindings(jl_binding_partition_t *owner, jl_binding_t *alias, size_t world) +static jl_binding_partition_t *jl_implicit_import_resolved(jl_binding_t *b, struct implicit_search_gap gap, struct implicit_search_resolution resolution) { - jl_binding_t *ownerb = NULL; - jl_binding_partition_t *alias_bpart = jl_get_binding_partition(alias, world); - if (owner == alias_bpart) - return 1; - jl_walk_binding_inplace(&ownerb, &owner, world); - jl_walk_binding_inplace(&alias, &alias_bpart, world); - if (jl_bkind_is_some_constant(jl_binding_kind(owner)) && - jl_bkind_is_some_constant(jl_binding_kind(alias_bpart)) && - owner->restriction && - alias_bpart->restriction == owner->restriction) - return 1; - return owner == alias_bpart; + jl_binding_partition_t *new_bpart = new_binding_partition(); + jl_atomic_store_relaxed(&new_bpart->max_world, gap.max_world < resolution.max_world ? gap.max_world : resolution.max_world); + new_bpart->min_world = gap.min_world > resolution.min_world ? gap.min_world : resolution.min_world; + new_bpart->kind = resolution.ultimate_kind | gap.inherited_flags; + new_bpart->restriction = resolution.binding_or_const; + jl_gc_wb_fresh(new_bpart, new_bpart->restriction); + jl_atomic_store_relaxed(&new_bpart->next, gap.replace); + if (!jl_atomic_cmpswap(gap.insert, &gap.replace, new_bpart)) + return NULL; + jl_gc_wb(gap.parent, new_bpart); + return new_bpart; } // find a binding from a module's `usings` list -void jl_check_new_binding_implicit( - jl_binding_partition_t *new_bpart, jl_binding_t *b, modstack_t *st, size_t world) -{ - modstack_t top = { b, st }; - modstack_t *tmp = st; - for (; tmp != NULL; tmp = tmp->prev) { - if (tmp->b == b) { - new_bpart->restriction = NULL; - new_bpart->kind = PARTITION_KIND_FAILED; /* PARTITION_KIND_CYCLE */ - return; +struct implicit_search_resolution jl_resolve_implicit_import(jl_binding_t *b, modstack_t *st, size_t world, int trust_cache) +{ + // First check if we've hit a cycle in this resolution + { + modstack_t *tmp = st; + for (; tmp != NULL; tmp = tmp->prev) { + if (tmp->b == b) { + return (struct implicit_search_resolution){ PARTITION_FAKE_KIND_CYCLE, NULL, 0, ~(size_t)0, 1, NULL, NULL }; + } } } jl_module_t *m = b->globalref->mod; jl_sym_t *var = b->globalref->name; - jl_binding_t *deprecated_impb = NULL; - jl_binding_t *impb = NULL; - jl_binding_partition_t *impbpart = NULL; - - size_t min_world = new_bpart->min_world; - size_t max_world = jl_atomic_load_relaxed(&new_bpart->max_world); + modstack_t top = { b, st }; + struct implicit_search_resolution impstate; + struct implicit_search_resolution depimpstate; + size_t min_world = 0; + size_t max_world = ~(size_t)0; + impstate = depimpstate = (struct implicit_search_resolution){ PARTITION_KIND_GUARD, NULL, min_world, max_world, 0, NULL, NULL }; JL_LOCK(&m->lock); int i = (int)module_usings_length(m) - 1; JL_UNLOCK(&m->lock); - enum jl_partition_kind guard_kind = PARTITION_KIND_GUARD; - for (; i >= 0; --i) { + for (; i >= 0 && impstate.ultimate_kind != PARTITION_KIND_FAILED; --i) { JL_LOCK(&m->lock); struct _jl_module_using data = *module_usings_getidx(m, i); JL_UNLOCK(&m->lock); @@ -87,144 +179,150 @@ void jl_check_new_binding_implicit( min_world = data.max_world + 1; continue; } + + min_world = WORLDMAX(min_world, data.min_world); + max_world = WORLDMIN(max_world, data.max_world); + jl_module_t *imp = data.mod; JL_GC_PROMISE_ROOTED(imp); jl_binding_t *tempb = jl_get_module_binding(imp, var, 0); - if (tempb != NULL) { - if (data.min_world > min_world) - min_world = data.min_world; - if (data.max_world < min_world) - max_world = data.max_world; - - jl_binding_partition_t *tempbpart = jl_get_binding_partition2(tempb, world, &top); - JL_GC_PROMISE_ROOTED(tempbpart); - - size_t tempbmax_world = jl_atomic_load_relaxed(&tempbpart->max_world); - if (tempbpart->min_world > min_world) - min_world = tempbpart->min_world; - if (tempbmax_world < max_world) - max_world = tempbmax_world; - - // N.B.: Which aspects of the partition are considered here needs to - // be kept in sync with `export_affecting_partition_flags` in the - // invalidation code. - if ((tempbpart->kind & PARTITION_FLAG_EXPORTED) == 0) - continue; + if (!tempb) { + // If the binding has never been allocated, it could not have been marked exported, so + // it is irrelevant for our resolution. We can move on. + continue; + } - if (impb) { - if (tempbpart->kind & PARTITION_FLAG_DEPRECATED) - continue; - if (jl_binding_kind(tempbpart) == PARTITION_KIND_GUARD && - jl_binding_kind(impbpart) != PARTITION_KIND_GUARD) - continue; - if (jl_binding_kind(impbpart) == PARTITION_KIND_GUARD) { - impb = tempb; - impbpart = tempbpart; - continue; - } - if (eq_bindings(tempbpart, impb, world)) - continue; - // Binding is ambiguous - // TODO: Even for eq bindings, this may need to further constrain the world age. - deprecated_impb = impb = NULL; - guard_kind = PARTITION_KIND_FAILED; - break; + struct implicit_search_gap gap; + jl_binding_partition_t *tempbpart = jl_get_binding_partition_if_present(tempb, world, &gap); + size_t tempbpart_flags = tempbpart ? (tempbpart->kind & PARTITION_MASK_FLAG) : gap.inherited_flags; + + while (tempbpart && jl_bkind_is_some_explicit_import(jl_binding_kind(tempbpart))) { + max_world = WORLDMIN(max_world, jl_atomic_load_relaxed(&tempbpart->max_world)); + min_world = WORLDMAX(min_world, tempbpart->min_world); + + tempb = (jl_binding_t*)tempbpart->restriction; + tempbpart = jl_get_binding_partition_if_present(tempb, world, &gap); + } + + int tempbpart_valid = tempbpart && (trust_cache || !jl_bkind_is_some_implicit(jl_binding_kind(tempbpart))); + size_t tembppart_max_world = tempbpart_valid ? jl_atomic_load_relaxed(&tempbpart->max_world) : gap.max_world; + size_t tembppart_min_world = tempbpart ? WORLDMAX(tempbpart->min_world, gap.min_world) : gap.min_world; + + max_world = WORLDMIN(max_world, tembppart_max_world); + min_world = WORLDMAX(min_world, tembppart_min_world); + + if (!(tempbpart_flags & PARTITION_FLAG_EXPORTED)) { + // Partition not exported - skip. + continue; + } + + struct implicit_search_resolution *comparison = &impstate; + if (impstate.ultimate_kind != PARTITION_KIND_GUARD) { + if (tempbpart_flags & PARTITION_FLAG_DEPRECATED) { + // Deprecated, but we already have a non-deprecated binding for this - skip. + continue; } - else if (tempbpart->kind & PARTITION_FLAG_DEPRECATED) { - if (deprecated_impb) { - if (!eq_bindings(tempbpart, deprecated_impb, world)) { - guard_kind = PARTITION_KIND_FAILED; - deprecated_impb = NULL; - } - } - else if (guard_kind == PARTITION_KIND_GUARD) { - deprecated_impb = tempb; - } + } else if (tempbpart_flags & PARTITION_FLAG_DEPRECATED) { + if (depimpstate.ultimate_kind == PARTITION_KIND_FAILED) { + // We've already decided that the deprecated bindings are ambiguous, so skip this, but + // keep going to look for non-deprecated bindings. + continue; } - else { - impb = tempb; - impbpart = tempbpart; + comparison = &depimpstate; + } + + struct implicit_search_resolution imp_resolution = { PARTITION_KIND_GUARD, NULL, min_world, max_world, 0, NULL, NULL }; + if (!tempbpart_valid) { + imp_resolution = jl_resolve_implicit_import(tempb, &top, world, trust_cache); + } else { + enum jl_partition_kind kind = jl_binding_kind(tempbpart); + if (kind == PARTITION_KIND_IMPLICIT_GLOBAL) { + imp_resolution.binding_or_const = tempbpart->restriction; + imp_resolution.debug_only_ultimate_binding = (jl_binding_t*)tempbpart->restriction; + imp_resolution.ultimate_kind = PARTITION_KIND_IMPLICIT_GLOBAL; + } else if (kind == PARTITION_KIND_GLOBAL || kind == PARTITION_KIND_DECLARED || kind == PARTITION_KIND_BACKDATED_CONST) { + imp_resolution.binding_or_const = (jl_value_t *)tempb; + imp_resolution.debug_only_ultimate_binding = tempb; + imp_resolution.ultimate_kind = PARTITION_KIND_IMPLICIT_GLOBAL; + } else if (jl_bkind_is_defined_constant(kind)) { + assert(tempbpart->restriction); + imp_resolution.binding_or_const = tempbpart->restriction; + imp_resolution.debug_only_ultimate_binding = tempb; + imp_resolution.ultimate_kind = PARTITION_KIND_IMPLICIT_CONST; } } - } + imp_resolution.debug_only_import_from = imp; + update_implicit_resolution(comparison, imp_resolution); - if (deprecated_impb && !impb) - impb = deprecated_impb; + if (!tempbpart && !imp_resolution.saw_cycle) { + // Independent of whether or not we trust the cache, we have independently computed the implicit resolution + // for this import, so we can put it in the cache. + jl_implicit_import_resolved(tempb, gap, imp_resolution); + } + } - assert(min_world <= max_world); - new_bpart->min_world = min_world; - jl_atomic_store_relaxed(&new_bpart->max_world, max_world); - if (impb) { - new_bpart->kind = PARTITION_KIND_IMPLICIT; - new_bpart->restriction = (jl_value_t*)impb; - jl_gc_wb(new_bpart, impb); - // TODO: World age constraints? - } else { - new_bpart->kind = guard_kind; - new_bpart->restriction = NULL; + if (impstate.ultimate_kind == PARTITION_KIND_GUARD && depimpstate.ultimate_kind != PARTITION_KIND_GUARD) { + depimpstate.min_world = WORLDMAX(depimpstate.min_world, min_world); + depimpstate.max_world = WORLDMIN(depimpstate.max_world, max_world); + return depimpstate; } + impstate.min_world = WORLDMAX(impstate.min_world, min_world); + impstate.max_world = WORLDMIN(impstate.max_world, max_world); + return impstate; } JL_DLLEXPORT jl_binding_partition_t *jl_maybe_reresolve_implicit(jl_binding_t *b, size_t new_max_world) { - jl_binding_partition_t *new_bpart = new_binding_partition(); - jl_binding_partition_t *bpart = jl_atomic_load_acquire(&b->partitions); - assert(bpart); - JL_GC_PUSH1(&new_bpart); + struct implicit_search_gap gap; while (1) { - jl_atomic_store_relaxed(&new_bpart->next, bpart); - jl_gc_wb(new_bpart, bpart); - jl_check_new_binding_implicit(new_bpart, b, NULL, new_max_world+1); - if (bpart->kind & PARTITION_FLAG_EXPORTED) - new_bpart->kind |= PARTITION_FLAG_EXPORTED; - if (new_bpart->kind == bpart->kind && new_bpart->restriction == bpart->restriction) { - JL_GC_POP(); + jl_binding_partition_t *bpart = jl_get_binding_partition_if_present(b, new_max_world+1, &gap); + assert(bpart == jl_atomic_load_relaxed(&b->partitions)); + assert(bpart); + struct implicit_search_resolution resolution = jl_resolve_implicit_import(b, NULL, new_max_world+1, 0); + if (resolution.min_world == bpart->min_world) { + assert(bpart->restriction == resolution.binding_or_const && jl_binding_kind(bpart) == resolution.ultimate_kind); return bpart; } - // Resolution changed, insert the new partition + assert(resolution.min_world == new_max_world+1 && "Missed an invalidation or bad resolution bounds"); size_t expected_max_world = ~(size_t)0; - if (jl_atomic_cmpswap(&bpart->max_world, &expected_max_world, new_max_world) && - jl_atomic_cmpswap(&b->partitions, &bpart, new_bpart)) { - jl_gc_wb(b, new_bpart); - JL_GC_POP(); - return new_bpart; + if (jl_atomic_cmpswap(&bpart->max_world, &expected_max_world, new_max_world)) + { + gap.min_world = new_max_world+1; + gap.inherited_flags = bpart->kind & PARTITION_MASK_FLAG; + jl_binding_partition_t *new_bpart = jl_implicit_import_resolved(b, gap, resolution); + if (new_bpart) + return new_bpart; } } } +JL_DLLEXPORT void jl_update_loaded_bpart(jl_binding_t *b, jl_binding_partition_t *bpart) +{ + struct implicit_search_resolution resolution = jl_resolve_implicit_import(b, NULL, jl_atomic_load_relaxed(&jl_world_counter), 0); + bpart->min_world = resolution.min_world; + jl_atomic_store_relaxed(&bpart->max_world, resolution.max_world); + bpart->restriction = resolution.binding_or_const; + bpart->kind = resolution.ultimate_kind; +} + STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b JL_PROPAGATES_ROOT, jl_value_t *parent, _Atomic(jl_binding_partition_t *)*insert, size_t world, modstack_t *st) JL_GLOBALLY_ROOTED { assert(jl_is_binding(b)); - jl_binding_partition_t *bpart = jl_atomic_load_relaxed(insert); - size_t max_world = (size_t)-1; - jl_binding_partition_t *new_bpart = NULL; - JL_GC_PUSH1(&new_bpart); + struct implicit_search_gap gap; + gap.parent = parent; + gap.insert = insert; + gap.inherited_flags = 0; + gap.min_world = 0; + gap.max_world = ~(size_t)0; while (1) { - while (bpart && world < bpart->min_world) { - insert = &bpart->next; - max_world = bpart->min_world - 1; - parent = (jl_value_t *)bpart; - bpart = jl_atomic_load_relaxed(&bpart->next); - } - if (bpart && world <= jl_atomic_load_relaxed(&bpart->max_world)) { - JL_GC_POP(); - return bpart; - } - if (!new_bpart) - new_bpart = new_binding_partition(); - jl_atomic_store_relaxed(&new_bpart->next, bpart); + gap.replace = jl_atomic_load_relaxed(gap.insert); + jl_binding_partition_t *bpart = jl_get_binding_partition__(b, world, &gap); if (bpart) - jl_gc_wb(new_bpart, bpart); // Not fresh the second time around the loop - new_bpart->min_world = bpart ? jl_atomic_load_relaxed(&bpart->max_world) + 1 : 0; - jl_atomic_store_relaxed(&new_bpart->max_world, max_world); - jl_check_new_binding_implicit(new_bpart, b, st, world); - if (bpart && (bpart->kind & PARTITION_FLAG_EXPORTED)) - new_bpart->kind |= PARTITION_FLAG_EXPORTED; - if (jl_atomic_cmpswap(insert, &bpart, new_bpart)) { - jl_gc_wb(parent, new_bpart); - JL_GC_POP(); + return bpart; + struct implicit_search_resolution resolution = jl_resolve_implicit_import(b, NULL, world, 1); + jl_binding_partition_t *new_bpart = jl_implicit_import_resolved(b, gap, resolution); + if (new_bpart) return new_bpart; - } } } @@ -242,11 +340,6 @@ jl_binding_partition_t *jl_get_binding_partition_with_hint(jl_binding_t *b, jl_b return jl_get_binding_partition_(b, (jl_value_t*)prev, &prev->next, world, NULL); } -jl_binding_partition_t *jl_get_binding_partition2(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world, modstack_t *st) JL_GLOBALLY_ROOTED { - assert(b); - return jl_get_binding_partition_(b, (jl_value_t*)b, &b->partitions, world, st); -} - jl_binding_partition_t *jl_get_binding_partition_all(jl_binding_t *b, size_t min_world, size_t max_world) { if (!b) return NULL; @@ -275,14 +368,17 @@ JL_DLLEXPORT int jl_get_binding_leaf_partitions_restriction_kind(jl_binding_t *b size_t cur_min_world = bpart->min_world; size_t cur_max_world = validated_min_world - 1; jl_walk_binding_inplace_worlds(&curb, &curbpart, &cur_min_world, &cur_max_world, &maybe_depwarn, cur_max_world); + enum jl_partition_kind kind = jl_binding_kind(curbpart); + if (kind == PARTITION_KIND_IMPLICIT_CONST) + kind = PARTITION_KIND_CONST; if (first == 1) { - rkp->kind = jl_binding_kind(curbpart); + rkp->kind = kind; rkp->restriction = curbpart->restriction; if (rkp->kind == PARTITION_KIND_GLOBAL || rkp->kind == PARTITION_KIND_DECLARED) rkp->binding_if_global = curb; first = 0; } else { - if (jl_binding_kind(curbpart) != rkp->kind || curbpart->restriction != rkp->restriction) + if (kind != rkp->kind || curbpart->restriction != rkp->restriction) return 0; if ((rkp->kind == PARTITION_KIND_GLOBAL || rkp->kind == PARTITION_KIND_DECLARED) && rkp->binding_if_global != curb) return 0; @@ -381,18 +477,18 @@ JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); while (!new_bpart) { enum jl_partition_kind kind = jl_binding_kind(bpart); - if (jl_bkind_is_some_constant(kind)) { + if (jl_bkind_is_some_constant(kind) && !jl_bkind_is_some_implicit(kind)) { if (!val) { new_bpart = bpart; break; } jl_value_t *old = bpart->restriction; JL_GC_PROMISE_ROOTED(old); - if (jl_egal(val, old)) { + if (val == old || (val && old && jl_egal(val, old))) { new_bpart = bpart; break; } - } else if (jl_bkind_is_some_import(kind) && kind != PARTITION_KIND_IMPLICIT) { + } else if (jl_bkind_is_some_explicit_import(kind)) { jl_errorf("cannot declare %s.%s constant; it was already declared as an import", jl_symbol_name(mod->name), jl_symbol_name(var)); } else if (kind == PARTITION_KIND_GLOBAL) { @@ -619,7 +715,7 @@ extern void check_safe_newbinding(jl_module_t *m, jl_sym_t *var) } } -static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b, jl_module_t *m, jl_sym_t *var) JL_GLOBALLY_ROOTED; +static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b) JL_GLOBALLY_ROOTED; // Checks that the binding in general is currently writable, but does not perform any checks on the // value to be written into the binding. @@ -638,15 +734,15 @@ JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module jl_symbol_name(m->name), jl_symbol_name(s), jl_symbol_name(s), jl_symbol_name(m->name)); } - else if (jl_bkind_is_some_constant(kind)) { + else if (jl_bkind_is_some_constant(kind) && kind != PARTITION_KIND_IMPLICIT_CONST) { jl_errorf("invalid assignment to constant %s.%s. This redefinition may be permitted using the `const` keyword.", jl_symbol_name(m->name), jl_symbol_name(s)); } else { - jl_module_t *from = jl_binding_dbgmodule(b, m, s); - if (from == m) + jl_module_t *from = jl_binding_dbgmodule(b); + if (from == m || !from) jl_errorf("cannot assign a value to imported variable %s.%s", - jl_symbol_name(from->name), jl_symbol_name(s)); + jl_symbol_name(m->name), jl_symbol_name(s)); else jl_errorf("cannot assign a value to imported variable %s.%s from module %s", jl_symbol_name(from->name), jl_symbol_name(s), jl_symbol_name(m->name)); @@ -667,6 +763,12 @@ JL_DLLEXPORT jl_module_t *jl_get_module_of_binding(jl_module_t *m, jl_sym_t *var jl_binding_t *b = jl_get_module_binding(m, var, 1); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); + if (jl_binding_kind(bpart) == PARTITION_KIND_IMPLICIT_CONST) { + struct implicit_search_resolution resolution = jl_resolve_implicit_import(b, NULL, jl_current_task->world_age, 0); + if (!resolution.debug_only_ultimate_binding) + jl_error("Constant binding was imported from multiple modules"); + b = resolution.debug_only_ultimate_binding; + } return b ? b->globalref->mod : m; } @@ -813,18 +915,37 @@ JL_DLLEXPORT jl_value_t *jl_bpart_get_restriction_value(jl_binding_partition_t * return v; } -// get binding for adding a method -// like jl_get_binding_wr, but has different error paths and messages -JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_t *var, size_t new_world) +// for error message printing: look up the module that exported a binding to m as var +// this might not be the same as the owner of the binding, since the binding itself may itself have been imported from elsewhere +static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b) +{ + jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); + enum jl_partition_kind kind = jl_binding_kind(bpart); + if (jl_bkind_is_some_explicit_import(kind) || kind == PARTITION_KIND_IMPLICIT_GLOBAL) { + return ((jl_binding_t*)bpart->restriction)->globalref->mod; + } + if (kind == PARTITION_KIND_IMPLICIT_CONST) { + struct implicit_search_resolution resolution = jl_resolve_implicit_import(b, NULL, jl_current_task->world_age, 1); + return resolution.debug_only_import_from; + } + return b->globalref->mod; +} + +// Look at the given binding and decide whether to add a new method to an existing generic function +// or ask for the creation of a new generic function (NULL return), checking various error conditions +// along the way. +JL_DLLEXPORT jl_value_t *jl_get_existing_strong_gf(jl_binding_t *b, size_t new_world) { - jl_binding_t *b = jl_get_module_binding(m, var, 1); jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world); enum jl_partition_kind kind = jl_binding_kind(bpart); - if (kind == PARTITION_KIND_GLOBAL || kind == PARTITION_KIND_DECLARED || jl_bkind_is_some_constant(kind)) - return b; - if (jl_bkind_is_some_guard(kind)) { - check_safe_newbinding(m, var); - return b; + if (jl_bkind_is_some_constant(kind) && kind != PARTITION_KIND_IMPLICIT_CONST) + return bpart->restriction; + if (jl_bkind_is_some_guard(kind) || kind == PARTITION_KIND_DECLARED) { + check_safe_newbinding(b->globalref->mod, b->globalref->name); + return NULL; + } + if (!jl_bkind_is_some_import(kind)) { + jl_errorf("cannot define function %s; it already has a value", jl_symbol_name(b->globalref->name)); } jl_binding_t *ownerb = b; jl_walk_binding_inplace(&ownerb, &bpart, new_world); @@ -832,35 +953,36 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ if (jl_bkind_is_some_constant(jl_binding_kind(bpart))) f = bpart->restriction; if (f == NULL) { - if (kind == PARTITION_KIND_IMPLICIT) { - check_safe_newbinding(m, var); - return b; + if (jl_bkind_is_some_implicit(kind)) { + check_safe_newbinding(b->globalref->mod, b->globalref->name); + return NULL; } - jl_module_t *from = jl_binding_dbgmodule(b, m, var); - // we must have implicitly imported this with using, so call jl_binding_dbgmodule to try to get the name of the module we got this from + jl_module_t *from = jl_binding_dbgmodule(b);\ + assert(from); // Can only be NULL if implicit, which we excluded above jl_errorf("invalid method definition in %s: exported function %s.%s does not exist", - jl_module_debug_name(m), jl_module_debug_name(from), jl_symbol_name(var)); + jl_module_debug_name(b->globalref->mod), jl_module_debug_name(from), jl_symbol_name(b->globalref->name)); } int istype = f && jl_is_type(f); if (!istype) { - if (kind == PARTITION_KIND_IMPLICIT) { - check_safe_newbinding(m, var); - return b; + if (jl_bkind_is_some_implicit(kind)) { + check_safe_newbinding(b->globalref->mod, b->globalref->name); + return NULL; } else if (kind != PARTITION_KIND_IMPORTED) { // TODO: we might want to require explicitly importing types to add constructors // or we might want to drop this error entirely - jl_module_t *from = jl_binding_dbgmodule(b, m, var); + jl_module_t *from = jl_binding_dbgmodule(b); + assert(from); // Can only be NULL if implicit, which we excluded above jl_errorf("invalid method definition in %s: function %s.%s must be explicitly imported to be extended", - jl_module_debug_name(m), jl_module_debug_name(from), jl_symbol_name(var)); + jl_module_debug_name(b->globalref->mod), jl_module_debug_name(from), jl_symbol_name(b->globalref->name)); } } else if (kind != PARTITION_KIND_IMPORTED) { - int should_error = strcmp(jl_symbol_name(var), "=>") == 0; - jl_module_t *from = jl_binding_dbgmodule(b, m, var); + int should_error = strcmp(jl_symbol_name(b->globalref->name), "=>") == 0; + jl_module_t *from = jl_binding_dbgmodule(b); if (should_error) { jl_errorf("invalid method definition in %s: function %s.%s must be explicitly imported to be extended", - jl_module_debug_name(m), jl_module_debug_name(from), jl_symbol_name(var)); + jl_module_debug_name(b->globalref->mod), from ? jl_module_debug_name(from) : "", jl_symbol_name(b->globalref->name)); } else if (!(jl_atomic_fetch_or_relaxed(&b->flags, BINDING_FLAG_DID_PRINT_IMPLICIT_IMPORT_ADMONITION) & BINDING_FLAG_DID_PRINT_IMPLICIT_IMPORT_ADMONITION)) { @@ -869,24 +991,13 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ " NOTE: This behavior may have differed in Julia versions prior to 1.12.\n" " Hint: If you intended to create a new generic function of the same name, use `function %s end`.\n" " Hint: To silence the warning, qualify `%s` as `%s.%s` in the method signature or explicitly `import %s: %s`.\n", - jl_symbol_name(var), jl_module_debug_name(m), - jl_symbol_name(var), jl_module_debug_name(from), jl_symbol_name(var), - jl_symbol_name(var), jl_symbol_name(var), jl_module_debug_name(from), jl_symbol_name(var), - jl_module_debug_name(from), jl_symbol_name(var)); + jl_symbol_name(b->globalref->name), jl_module_debug_name(b->globalref->mod), + jl_symbol_name(b->globalref->name), jl_module_debug_name(from), jl_symbol_name(b->globalref->name), + jl_symbol_name(b->globalref->name), jl_symbol_name(b->globalref->name), jl_module_debug_name(from), jl_symbol_name(b->globalref->name), + jl_module_debug_name(from), jl_symbol_name(b->globalref->name)); } } - return ownerb; -} - -// for error message printing: look up the module that exported a binding to m as var -// this might not be the same as the owner of the binding, since the binding itself may itself have been imported from elsewhere -static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b, jl_module_t *m, jl_sym_t *var) -{ - jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - if (jl_bkind_is_some_import(jl_binding_kind(bpart))) { - return ((jl_binding_t*)bpart->restriction)->globalref->mod; - } - return m; + return f; } static void jl_binding_dep_message(jl_binding_t *b); @@ -995,6 +1106,22 @@ JL_DLLEXPORT void check_safe_import_from(jl_module_t *m) } } +static int eq_bindings(jl_binding_partition_t *owner, jl_binding_t *alias, size_t world) +{ + jl_binding_t *ownerb = NULL; + jl_binding_partition_t *alias_bpart = jl_get_binding_partition(alias, world); + if (owner == alias_bpart) + return 1; + jl_walk_binding_inplace(&ownerb, &owner, world); + jl_walk_binding_inplace(&alias, &alias_bpart, world); + if (jl_bkind_is_some_constant(jl_binding_kind(owner)) && + jl_bkind_is_some_constant(jl_binding_kind(alias_bpart)) && + owner->restriction && + alias_bpart->restriction == owner->restriction) + return 1; + return owner == alias_bpart; +} + // NOTE: we use explici since explicit is a C++ keyword static void module_import_(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici) { @@ -1148,7 +1275,7 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from) if (tobpart) { enum jl_partition_kind kind = jl_binding_kind(tobpart); if (jl_bkind_is_some_implicit(kind)) { - jl_replace_binding_locked(tob, tobpart, NULL, PARTITION_KIND_IMPLICIT_RECOMPUTE, new_world); + jl_replace_binding_locked(tob, tobpart, NULL, PARTITION_FAKE_KIND_IMPLICIT_RECOMPUTE, new_world); } } } @@ -1437,10 +1564,12 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, jl_binding_partition_t *new_bpart = new_binding_partition(); JL_GC_PUSH1(&new_bpart); new_bpart->min_world = new_world; - if ((kind & PARTITION_MASK_KIND) == PARTITION_KIND_IMPLICIT_RECOMPUTE) { + if ((kind & PARTITION_MASK_KIND) == PARTITION_FAKE_KIND_IMPLICIT_RECOMPUTE) { assert(!restriction_val); - jl_check_new_binding_implicit(new_bpart, b, NULL, new_world); - new_bpart->kind |= kind & PARTITION_MASK_FLAG; + struct implicit_search_resolution resolution = jl_resolve_implicit_import(b, NULL, new_world, 0); + new_bpart->kind = resolution.ultimate_kind | (kind & PARTITION_MASK_FLAG); + new_bpart->restriction = resolution.binding_or_const; + assert(resolution.min_world <= new_world && resolution.max_world == ~(size_t)0); if (new_bpart->kind == old_bpart->kind && new_bpart->restriction == old_bpart->restriction) { JL_GC_POP(); return old_bpart; @@ -1561,14 +1690,9 @@ static int should_depwarn(jl_binding_t *b, uint8_t flag) // (`using` or `import`). The logic here is that the thing that needs to be adjusted // is not the use itself, but rather the `using` or `import` (which already prints // an appropriate warning). - for (;;) { - jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - if (bpart->kind & PARTITION_FLAG_DEPRECATED) - return 1; - if ((bpart->kind & PARTITION_MASK_KIND) != PARTITION_KIND_IMPLICIT) - break; - b = (jl_binding_t*)bpart->restriction; - } + jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); + if (bpart->kind & flag) + return 1; return 0; } @@ -1592,18 +1716,6 @@ void jl_binding_deprecation_warning(jl_binding_t *b) jl_printf(JL_STDERR, "WARNING: "); jl_printf(JL_STDERR, "Use of "); - jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - int first = 0; - while (!(bpart->kind & PARTITION_FLAG_DEPWARN)) { - if (first) { - jl_printf(JL_STDERR, "binding implicitly imported via "); - first = 0; - } - jl_printf(JL_STDERR, "%s.%s -> ", jl_symbol_name(b->globalref->mod->name), jl_symbol_name(b->globalref->name)); - assert(jl_binding_kind(bpart) == PARTITION_KIND_IMPLICIT); - b = (jl_binding_t*)bpart->restriction; - bpart = jl_get_binding_partition(b, jl_current_task->world_age); - } jl_printf(JL_STDERR, "%s.%s is deprecated", jl_symbol_name(b->globalref->mod->name), jl_symbol_name(b->globalref->name)); jl_binding_dep_message(b); @@ -1807,7 +1919,7 @@ JL_DLLEXPORT void jl_clear_implicit_imports(jl_module_t *m) if ((void*)b == jl_nothing) break; jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); - if (jl_binding_kind(bpart) == PARTITION_KIND_IMPLICIT) { + if (jl_bkind_is_some_implicit(jl_binding_kind(bpart))) { jl_atomic_store_relaxed(&b->partitions, NULL); } } diff --git a/src/staticdata.c b/src/staticdata.c index e42b303a328ce..d3d1cc91e9aa2 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -3582,12 +3582,14 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t size_t raw_kind = bpart->kind; enum jl_partition_kind kind = (enum jl_partition_kind)(raw_kind & PARTITION_MASK_KIND); if (!unchanged_implicit && jl_bkind_is_some_implicit(kind)) { - jl_check_new_binding_implicit(bpart, b, NULL, jl_atomic_load_relaxed(&jl_world_counter)); + // TODO: Should we actually update this in place or delete it from the partitions list + // and allocate a fresh bpart? + jl_update_loaded_bpart(b, bpart); bpart->kind |= (raw_kind & PARTITION_MASK_FLAG); if (bpart->min_world > jl_require_world) goto invalidated; } - if (!jl_bkind_is_some_import(kind)) + if (!jl_bkind_is_some_explicit_import(kind) && kind != PARTITION_KIND_IMPLICIT_GLOBAL) return 1; jl_binding_t *imported_binding = (jl_binding_t*)bpart->restriction; if (no_replacement) diff --git a/src/toplevel.c b/src/toplevel.c index 35ec0684ab7a7..73b65ecf2e3fb 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -311,7 +311,7 @@ void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type, in bpart = jl_get_binding_partition(b, new_world); enum jl_partition_kind kind = jl_binding_kind(bpart); if (kind != PARTITION_KIND_GLOBAL) { - if (jl_bkind_is_some_guard(kind) || kind == PARTITION_KIND_DECLARED || kind == PARTITION_KIND_IMPLICIT) { + if (jl_bkind_is_some_implicit(kind) || kind == PARTITION_KIND_DECLARED) { if (kind == new_kind) { if (!set_type) goto done; @@ -659,9 +659,9 @@ static void import_module(jl_task_t *ct, jl_module_t *JL_NONNULL m, jl_module_t jl_binding_t *b = jl_get_module_binding(m, name, 1); jl_binding_partition_t *bpart = jl_get_binding_partition(b, ct->world_age); enum jl_partition_kind kind = jl_binding_kind(bpart); - if (kind != PARTITION_KIND_GUARD && kind != PARTITION_KIND_FAILED && kind != PARTITION_KIND_DECLARED && kind != PARTITION_KIND_IMPLICIT) { + if (!jl_bkind_is_some_implicit(kind) && kind != PARTITION_KIND_DECLARED) { // Unlike regular constant declaration, we allow this as long as we eventually end up at a constant. - jl_walk_binding_inplace(&b, &bpart, ct->world_age); + jl_walk_binding_inplace(&b, &bpart, ct->world_age); if (jl_binding_kind(bpart) == PARTITION_KIND_CONST || jl_binding_kind(bpart) == PARTITION_KIND_BACKDATED_CONST || jl_binding_kind(bpart) == PARTITION_KIND_CONST_IMPORT) { // Already declared (e.g. on another thread) or imported. if (bpart->restriction == (jl_value_t*)import) diff --git a/test/core.jl b/test/core.jl index 7bf34da3076d9..7e6749d662503 100644 --- a/test/core.jl +++ b/test/core.jl @@ -6120,7 +6120,6 @@ module GlobalDef18933 global sincos nothing end - @test which(@__MODULE__, :sincos) === Base.Math @test @isdefined sincos @test sincos === Base.sincos end @@ -8478,3 +8477,54 @@ module GlobalAssign57446 (@__MODULE__).theglobal = 1 @test theglobal == 1 end + +# issue #57638 - circular imports +module M57638 +module I + using ..M57638 +end +using .I +end +convert(Core.Binding, GlobalRef(M57638.I, :Base)) +@test M57638.Base === Base + +module M57638_2 +module I + using ..M57638_2 + export Base +end +using .I +export Base +end +@test M57638_2.Base === Base + +module M57638_3 + module M2 + using ..M57638_3 + module M3 + const x = 1 + export x + end + using .M3 + export x + end + using .M2 + export x +end +@test M57638_3.x === 1 + +module GlobalBindingMulti + module M + export S + module C + export S + struct A end + S = A() # making S const makes the error go away + end + using .C + end + + using .M + using .M.C +end +@test GlobalBindingMulti.S === GlobalBindingMulti.M.C.S diff --git a/test/reflection.jl b/test/reflection.jl index a0e9d96f044be..f5681c23777b1 100644 --- a/test/reflection.jl +++ b/test/reflection.jl @@ -1295,3 +1295,24 @@ end @test Base.infer_return_type(code_lowered, (Any,Any)) == Vector{Core.CodeInfo} @test methods(Union{}) == Any[m.method for m in Base._methods_by_ftype(Tuple{Core.TypeofBottom, Vararg}, 1, Base.get_world_counter())] # issue #55187 + +# which should not look through const bindings, even if they have the same value +# as a previous implicit import +module SinConst +const sin = Base.sin +end + +@test which(SinConst, :sin) === SinConst + +# `which` should error if there is not a unique binding that a constant was imported from +module X1ConstConflict +const xconstconflict = 1 +export xconstconflict +end +module X2ConstConflict +const xconstconflict = 1 +export xconstconflict +end +using .X1ConstConflict, .X2ConstConflict + +@test_throws ErrorException which(@__MODULE__, :xconstconflict)