Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

print admonition for auto-import only once per module #57378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

print admonition for auto-import only once per module #57378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,10 +741,11 @@ typedef struct _jl_binding_t {
_Atomic(jl_binding_partition_t*) partitions;
jl_array_t *backedges;
uint8_t did_print_backdate_admonition:1;
uint8_t did_print_implicit_import_admonition:1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/JuliaLang/julia/blob/master/base/invalidation.jl#L116 currently assumes the bitposition of the export flag which this changes. I'm about to refactor that code to no longer use the bitset (which is implementation defined anyway), but if you want to merge this change before then, you'll have to adjust one or the other.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. I will probably just fix the offset for now. Immediately after this PR, I want to change this to an atomics flags field so we can use atomic-and / atomic-or to set these flags safely.

uint8_t exportp:1; // `public foo` sets `publicp`, `export foo` sets both `publicp` and `exportp`
uint8_t publicp:1; // exportp without publicp is not allowed.
uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package
uint8_t padding:3;
uint8_t padding:2;
} jl_binding_t;

typedef struct {
Expand Down
10 changes: 7 additions & 3 deletions src/module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name)
b->publicp = 0;
b->deprecated = 0;
b->did_print_backdate_admonition = 0;
b->did_print_implicit_import_admonition = 0;
JL_GC_PUSH1(&b);
b->globalref = jl_new_globalref(mod, name, b);
jl_gc_wb(b, b->globalref);
Expand Down Expand Up @@ -458,14 +459,14 @@ JL_DLLEXPORT jl_module_t *jl_get_module_of_binding(jl_module_t *m, jl_sym_t *var

static NOINLINE void print_backdate_admonition(jl_binding_t *b) JL_NOTSAFEPOINT
{
b->did_print_backdate_admonition = 1;
jl_safe_printf(
"WARNING: Detected access to binding `%s.%s` in a world prior to its definition world.\n"
" Julia 1.12 has introduced more strict world age semantics for global bindings.\n"
" !!! This code may malfunction under Revise.\n"
" !!! This code will error in future versions of Julia.\n"
"Hint: Add an appropriate `invokelatest` around the access to this binding.\n",
jl_symbol_name(b->globalref->mod->name), jl_symbol_name(b->globalref->name));
b->did_print_backdate_admonition = 1;
}

static inline void check_backdated_binding(jl_binding_t *b, enum jl_partition_kind kind) JL_NOTSAFEPOINT
Expand Down Expand Up @@ -632,10 +633,12 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
else if (kind != BINDING_KIND_IMPORTED) {
int should_error = strcmp(jl_symbol_name(var), "=>") == 0;
jl_module_t *from = jl_binding_dbgmodule(b, m, var);
if (should_error)
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));
else
}
else if (!b->did_print_implicit_import_admonition) {
b->did_print_implicit_import_admonition = 1;
jl_printf(JL_STDERR, "WARNING: Constructor for type \"%s\" was extended in `%s` without explicit qualification or import.\n"
" NOTE: Assumed \"%s\" refers to `%s.%s`. This behavior is deprecated and may differ in future versions.`\n"
" NOTE: This behavior may have differed in Julia versions prior to 1.12.\n"
Expand All @@ -645,6 +648,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_
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));
}
}
return ownerb;
}
Expand Down