Skip to content

Commit 294b765

Browse files
committed
Add a warning for auto-import of types
This adds a warning for the auto-import of types cases (#25744) that we have long considered a bit of a bug, but didn't want to change because it is too breaking. The reason to do it now is that the binding rework has made this case more problematic (see #57290). To summarize, the question is what happens when the compiler sees `f(x) = ...` and `f` is currently and implicitly imported binding. There are two options: 1. We add a method to the generic function referred to by `f`, or 2. We create a new generic function `f` in the current module. Historically, case 1 has the additional complication that this error'd unless `f` is a type. It is my impression that a lot of existing code did not have a particularly good understanding of the resolved-ness dependence of this behavior. However, because case 1 errors for generic functions, it appears that existing code generally expects case 2. On the other hand, for types, there is existing code in both directions (#57290 is an example of case 2; see #57302 for examples of case 1). That said, case 1 is more common (because types tend to be resolved because they're used in signatures at toplevel). Thus, to retain compatibility, the current behavior on master (where resolvedness is no longer available) is that we always choose case 2 for functions and case 1 for types. This inconsistency is unfortunate, but I tried resolving this in either way (making all situations case 1 or all case 2) and the result was too breaking. Nevertheless, it is problematic that there is existing code that expects case 2 beavior for types and we should help users to know what the correct way to fix it is. The proposed resolution is thus: 1. Retain case 1 behavior for types 2. Make it a warning to use, encouraging people to explicitly import, since we generally consider the #25744 case a bug. Example: ``` julia> module Foo String(i::Int) = i end WARNING: Type Core.String was auto-`import`ed in `Foo`. NOTE: This behavior is deprecated and may change in future Julia versions. NOTE: This behavior may have differed in Julia versions prior to 1.12 depending on binding resolution. Hint: To retain the current behavior, add an explicit `import Core: String` in Foo. Hint: To create a new generic function of the same name use `function String end`. Main.Foo ```
1 parent c269329 commit 294b765

10 files changed

+41
-37
lines changed

src/codegen.cpp

+4-21
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,7 @@ static const auto jlgenericfunction_func = new JuliaFunction<>{
10051005
auto T_jlvalue = JuliaType::get_jlvalue_ty(C);
10061006
auto T_pjlvalue = PointerType::get(T_jlvalue, 0);
10071007
auto T_prjlvalue = PointerType::get(T_jlvalue, AddressSpace::Tracked);
1008-
return FunctionType::get(T_prjlvalue, {T_pjlvalue, T_pjlvalue, T_pjlvalue}, false);
1008+
return FunctionType::get(T_prjlvalue, {T_pjlvalue, T_pjlvalue}, false);
10091009
},
10101010
nullptr,
10111011
};
@@ -6853,8 +6853,6 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
68536853
jl_value_t *mn = args[0];
68546854
assert(jl_is_symbol(mn) || jl_is_slotnumber(mn) || jl_is_globalref(mn));
68556855

6856-
Value *bp = NULL, *name;
6857-
jl_binding_t *bnd = NULL;
68586856
bool issym = jl_is_symbol(mn);
68596857
bool isglobalref = !issym && jl_is_globalref(mn);
68606858
jl_module_t *mod = ctx.module;
@@ -6863,26 +6861,11 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
68636861
mod = jl_globalref_mod(mn);
68646862
mn = (jl_value_t*)jl_globalref_name(mn);
68656863
}
6866-
JL_TRY {
6867-
if (jl_symbol_name((jl_sym_t*)mn)[0] == '@')
6868-
jl_errorf("macro definition not allowed inside a local scope");
6869-
name = literal_pointer_val(ctx, mn);
6870-
bnd = jl_get_binding_for_method_def(mod, (jl_sym_t*)mn);
6871-
}
6872-
JL_CATCH {
6873-
jl_value_t *e = jl_current_exception(jl_current_task);
6874-
// errors. boo. :(
6875-
JL_GC_PUSH1(&e);
6876-
e = jl_as_global_root(e, 1);
6877-
JL_GC_POP();
6878-
raise_exception(ctx, literal_pointer_val(ctx, e));
6879-
return ghostValue(ctx, jl_nothing_type);
6880-
}
6881-
bp = julia_binding_gv(ctx, bnd);
68826864
jl_cgval_t gf = mark_julia_type(
68836865
ctx,
6884-
ctx.builder.CreateCall(prepare_call(jlgenericfunction_func), { bp,
6885-
literal_pointer_val(ctx, (jl_value_t*)mod), name
6866+
ctx.builder.CreateCall(prepare_call(jlgenericfunction_func), {
6867+
literal_pointer_val(ctx, (jl_value_t*)mod),
6868+
literal_pointer_val(ctx, (jl_value_t*)mn)
68866869
}),
68876870
true,
68886871
jl_function_type);

src/interpreter.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ static jl_value_t *eval_methoddef(jl_expr_t *ex, interpreter_state *s)
9393
if (!jl_is_symbol(fname)) {
9494
jl_error("method: invalid declaration");
9595
}
96-
jl_binding_t *b = jl_get_binding_for_method_def(modu, fname);
97-
return jl_declare_const_gf(b, modu, fname);
96+
return jl_declare_const_gf(modu, fname);
9897
}
9998

10099
jl_value_t *atypes = NULL, *meth = NULL, *fname = NULL;

src/julia.h

+9-2
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,13 @@ STATIC_INLINE char *jl_symbol_name_(jl_sym_t *s) JL_NOTSAFEPOINT
14751475
}
14761476
#define jl_symbol_name(s) jl_symbol_name_(s)
14771477

1478+
STATIC_INLINE const char *jl_module_debug_name(jl_module_t *mod) JL_NOTSAFEPOINT
1479+
{
1480+
if (!mod)
1481+
return "<null>";
1482+
return jl_symbol_name(mod->name);
1483+
}
1484+
14781485
static inline uint32_t jl_fielddesc_size(int8_t fielddesc_type) JL_NOTSAFEPOINT
14791486
{
14801487
assert(fielddesc_type >= 0 && fielddesc_type <= 2);
@@ -1901,7 +1908,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT
19011908
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT);
19021909
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
19031910
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_and_const(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
1904-
JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_binding_t *b, jl_module_t *mod, jl_sym_t *name);
1911+
JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_module_t *mod, jl_sym_t *name);
19051912
JL_DLLEXPORT jl_method_t *jl_method_def(jl_svec_t *argdata, jl_methtable_t *mt, jl_code_info_t *f, jl_module_t *module);
19061913
JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo, size_t world, jl_code_instance_t **cache);
19071914
JL_DLLEXPORT jl_code_info_t *jl_copy_code_info(jl_code_info_t *src);
@@ -2051,7 +2058,7 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var);
20512058
// get binding for assignment
20522059
JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module_t *m, jl_sym_t *s);
20532060
JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
2054-
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var);
2061+
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);
20552062
JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var, int allow_import);
20562063
JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var);
20572064
JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var);

src/julia_internal.h

+1
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,7 @@ STATIC_INLINE size_t module_usings_max(jl_module_t *m) JL_NOTSAFEPOINT {
876876
return m->usings.max/3;
877877
}
878878

879+
JL_DLLEXPORT jl_sym_t *jl_module_name(jl_module_t *m) JL_NOTSAFEPOINT;
879880
jl_value_t *jl_eval_global_var(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *e);
880881
jl_value_t *jl_interpret_opaque_closure(jl_opaque_closure_t *clos, jl_value_t **args, size_t nargs);
881882
jl_value_t *jl_interpret_toplevel_thunk(jl_module_t *m, jl_code_info_t *src);

src/method.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -1050,10 +1050,11 @@ JL_DLLEXPORT void jl_check_gf(jl_value_t *gf, jl_sym_t *name)
10501050
jl_errorf("cannot define function %s; it already has a value", jl_symbol_name(name));
10511051
}
10521052

1053-
JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_binding_t *b, jl_module_t *mod, jl_sym_t *name)
1053+
JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_module_t *mod, jl_sym_t *name)
10541054
{
10551055
JL_LOCK(&world_counter_lock);
10561056
size_t new_world = jl_atomic_load_relaxed(&jl_world_counter) + 1;
1057+
jl_binding_t *b = jl_get_binding_for_method_def(mod, name, new_world);
10571058
jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world);
10581059
jl_ptr_kind_union_t pku = jl_atomic_load_relaxed(&bpart->restriction);
10591060
jl_value_t *gf = NULL;

src/module.c

+20-8
Original file line numberDiff line numberDiff line change
@@ -588,10 +588,10 @@ JL_DLLEXPORT jl_value_t *jl_reresolve_binding_value_seqcst(jl_binding_t *b)
588588

589589
// get binding for adding a method
590590
// like jl_get_binding_wr, but has different error paths and messages
591-
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_t *var)
591+
JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_t *var, size_t new_world)
592592
{
593593
jl_binding_t *b = jl_get_module_binding(m, var, 1);
594-
jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
594+
jl_binding_partition_t *bpart = jl_get_binding_partition(b, new_world);
595595
jl_ptr_kind_union_t pku = jl_atomic_load_relaxed(&bpart->restriction);
596596
enum jl_partition_kind kind = decode_restriction_kind(pku);
597597
if (kind == BINDING_KIND_GLOBAL || kind == BINDING_KIND_DECLARED || jl_bkind_is_some_constant(decode_restriction_kind(pku)))
@@ -601,7 +601,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
601601
return b;
602602
}
603603
jl_binding_t *ownerb = b;
604-
pku = jl_walk_binding_inplace(&ownerb, &bpart, jl_current_task->world_age);
604+
pku = jl_walk_binding_inplace(&ownerb, &bpart, new_world);
605605
jl_value_t *f = NULL;
606606
if (jl_bkind_is_some_constant(decode_restriction_kind(pku)))
607607
f = decode_restriction_value(pku);
@@ -613,7 +613,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
613613
jl_module_t *from = jl_binding_dbgmodule(b, m, var);
614614
// 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
615615
jl_errorf("invalid method definition in %s: exported function %s.%s does not exist",
616-
jl_symbol_name(m->name), from ? jl_symbol_name(from->name) : "<null>", jl_symbol_name(var));
616+
jl_module_debug_name(m), jl_module_debug_name(from), jl_symbol_name(var));
617617
}
618618
int istype = f && jl_is_type(f);
619619
if (!istype) {
@@ -626,13 +626,25 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
626626
// or we might want to drop this error entirely
627627
jl_module_t *from = jl_binding_dbgmodule(b, m, var);
628628
jl_errorf("invalid method definition in %s: function %s.%s must be explicitly imported to be extended",
629-
jl_symbol_name(m->name), from ? jl_symbol_name(from->name) : "<null>", jl_symbol_name(var));
629+
jl_module_debug_name(m), jl_module_debug_name(from), jl_symbol_name(var));
630630
}
631631
}
632-
else if (strcmp(jl_symbol_name(var), "=>") == 0 && (kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_EXPLICIT)) {
632+
else if (kind != BINDING_KIND_IMPORTED) {
633+
int should_error = strcmp(jl_symbol_name(var), "=>") == 0;
633634
jl_module_t *from = jl_binding_dbgmodule(b, m, var);
634-
jl_errorf("invalid method definition in %s: function %s.%s must be explicitly imported to be extended",
635-
jl_symbol_name(m->name), from ? jl_symbol_name(from->name) : "<null>", jl_symbol_name(var));
635+
if (should_error)
636+
jl_errorf("invalid method definition in %s: function %s.%s must be explicitly imported to be extended",
637+
jl_module_debug_name(m), jl_module_debug_name(from), jl_symbol_name(var));
638+
else
639+
jl_printf(JL_STDERR, "WARNING: Constructor for type \"%s\" was extended in `%s` without explicit qualification or import.\n"
640+
" NOTE: Assumed \"%s\" refers to `%s.%s`. This behavior is deprecated and may differ in future versions.`\n"
641+
" NOTE: This behavior may have differed in Julia versions prior to 1.12.\n"
642+
" Hint: If you intended to create a new generic function of the same name, use `function %s end`.\n"
643+
" Hint: To silence the warning, qualify `%s` as `%s.%s` or explicitly `import %s: %s`\n",
644+
jl_symbol_name(var), jl_module_debug_name(m),
645+
jl_symbol_name(var), jl_module_debug_name(from), jl_symbol_name(var),
646+
jl_symbol_name(var), jl_symbol_name(var), jl_module_debug_name(from), jl_symbol_name(var),
647+
jl_module_debug_name(from), jl_symbol_name(var));
636648
}
637649
return ownerb;
638650
}

stdlib/SharedArrays/src/SharedArrays.jl

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ using Mmap, Distributed, Random
99

1010
import Base: length, size, elsize, ndims, IndexStyle, reshape, convert, deepcopy_internal,
1111
show, getindex, setindex!, fill!, similar, reduce, map!, copyto!, cconvert
12+
import Base: Array
1213
import Random
1314
using Serialization
1415
using Serialization: serialize_cycle_header, serialize_type, writetag, UNDEFREF_TAG, serialize, deserialize

test/arrayops.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -2948,7 +2948,7 @@ end
29482948

29492949
Base.ArithmeticStyle(::Type{F21666{T}}) where {T} = T()
29502950
Base.:+(x::F, y::F) where {F <: F21666} = F(x.x + y.x)
2951-
Float64(x::F21666) = Float64(x.x)
2951+
Base.Float64(x::F21666) = Float64(x.x)
29522952
@testset "Exactness of cumsum # 21666" begin
29532953
# test that cumsum uses more stable algorithm
29542954
# for types with unknown/rounding arithmetic

test/errorshow.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ let err_str
438438
@test occursin("For element-wise subtraction, use broadcasting with dot syntax: array .- scalar", err_str)
439439
end
440440

441-
441+
import Core: String
442442
method_defs_lineno = @__LINE__() + 1
443443
String() = throw(ErrorException("1"))
444444
(::String)() = throw(ErrorException("2"))

test/intrinsics.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ end
6666
# test functionality of non-power-of-2 primitive type constants
6767
primitive type Int24 24 end
6868
Int24(x::Int) = Core.Intrinsics.trunc_int(Int24, x)
69-
Int(x::Int24) = Core.Intrinsics.zext_int(Int, x)
69+
Base.Int(x::Int24) = Core.Intrinsics.zext_int(Int, x)
7070
let x, y, f
7171
x = Int24(Int(0x12345678)) # create something (via truncation)
7272
@test Int(0x345678) === Int(x)

0 commit comments

Comments
 (0)