Skip to content

print_backdate_admonition may need a backtrace #57969

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.

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

Closed
timholy opened this issue Apr 1, 2025 · 5 comments · Fixed by #58266
Closed

print_backdate_admonition may need a backtrace #57969

timholy opened this issue Apr 1, 2025 · 5 comments · Fixed by #58266
Labels
triage This should be discussed on a triage call
Milestone

Comments

@timholy
Copy link
Member

timholy commented Apr 1, 2025

The binding partitioning work introduced stricter rules for accessing globals, including a scary warning from

julia/src/module.c

Lines 775 to 784 in 13311f3

static NOINLINE void print_backdate_admonition(jl_binding_t *b) JL_NOTSAFEPOINT
{
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));
}
My experience is that it's been quite difficult to pinpoint the problematic line without a backtrace. Another option would be to have an option to turn the warning into an error.

@timholy timholy added this to the 1.12 milestone Apr 1, 2025
@vtjnash
Copy link
Member

vtjnash commented Apr 1, 2025

How strict do we want to be? We could do this:

diff --git a/src/module.c b/src/module.c
index 7d77e933f81..a54384967aa 100644
--- a/src/module.c
+++ b/src/module.c
@@ -773,6 +773,8 @@ 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
 {
+    if (jl_options.depwarn == JL_OPTIONS_DEPWARN_ERROR)
+        jl_undefined_var_error(b->globalref->name, (jl_value_t*)b->globalref->mod);
     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"
diff --git a/base/errorshow.jl b/base/errorshow.jl
index c53c605cbfb..b1b7a083eb0 100644
--- a/base/errorshow.jl
+++ b/base/errorshow.jl
@@ -1163,6 +1163,8 @@ function UndefVarError_hint(io::IO, ex::UndefVarError)
                 print(io, "\nSuggestion: check for spelling errors or missing imports.")
             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.")
+            elseif kind === Base.PARTITION_KIND_BACKDATED_CONST
+                print(io, "\nSuggestion: this const was attempted to be used before it was defined (new Julia v1.12 rule).")
             end
         elseif scope === :static_parameter
             print(io, "\nSuggestion: run Test.detect_unbound_args to detect method arguments that do not fully constrain a type parameter.")

resulting in:

$ ./julia --depwarn=error -E 't = @task x; const x = 4; fetch(schedule(t))'
ERROR: TaskFailedException
Stacktrace:
 [1] #wait#581
   @ ./task.jl:363 [inlined]
 [2] wait
   @ ./task.jl:360 [inlined]
 [3] fetch(t::Task)
   @ Base ./task.jl:525
 [4] top-level scope
   @ none:1
 [5] eval(m::Module, e::Any)
   @ Core ./boot.jl:489
 [6] exec_options(opts::Base.JLOptions)
   @ Base ./client.jl:296
 [7] _start()
   @ Base ./client.jl:560

    nested task error: UndefVarError: `x` not defined in `Main`
    Suggestion: this const was attempted to be used before it was defined (new Julia v1.12 rule).
    Stacktrace:
     [1] (::var"#5#6")()
       @ Main ./none:1

@timholy
Copy link
Member Author

timholy commented Apr 2, 2025

I like it. The only minor weakness I see with this proposal is that you can't get lineinfo unless you make it an error, so it's not possible to use it like @depwarn which (nominally) gives line info even when it's just a warning. That said, I'd say the convenience is minor, and (at least as of Julia 1.10) the depwarn line number printing is still sufficiently hit-or-miss that I often turn them into errors anyway.

Possible alternate phrasing: "Suggestion: define the constant before running code that uses it (stricter Julia v1.12+ rule)."

@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2025

It looks like the hint code is also not currently supporting binding ages like method printing does (to alert you if there is a binding in a different world with different values) which is likely a separate issue with the resulting hint text

@vtjnash vtjnash added the triage This should be discussed on a triage call label Apr 2, 2025
@vtjnash
Copy link
Member

vtjnash commented Apr 2, 2025

I marked for triage/needs decision, since I want folks to be on-board with making this controlled by the existing depwarn error flag before we commit to doing that

@Keno
Copy link
Member

Keno commented Apr 2, 2025

I do think the UndefVarError one is reasonable, because that's closest to the behavior you'll get after we remove the deprecation.

vtjnash added a commit that referenced this issue Apr 29, 2025
This is close to the expected behavior after deprecations are removed
(other than that the b->globalref->mod in the printed message here will
be the source module instead of the destination module, which may
sometimes cause confusing printing here, but probably rarely).

Closes #57969
vtjnash added a commit that referenced this issue Apr 29, 2025
Wording updated based upon timholy's suggestion in #57969.
vtjnash added a commit that referenced this issue Apr 29, 2025
This is close to the expected behavior after deprecations are removed
(other than that the b->globalref->mod in the printed message here will
be the source module instead of the destination module, which may
sometimes cause confusing printing here, but probably rarely).

Closes #57969
KristofferC pushed a commit that referenced this issue Apr 29, 2025
Wording updated based upon timholy's suggestion in #57969.

(cherry picked from commit 1aeea19)
vtjnash added a commit that referenced this issue May 2, 2025
This is close to the expected behavior after deprecations are removed
(other than that the b->globalref->mod in the printed message here will
be the source module instead of the destination module, which may
sometimes cause confusing printing here, but probably rarely).

Closes #57969
@Keno Keno closed this as completed in 0682158 May 3, 2025
KristofferC pushed a commit that referenced this issue May 5, 2025
This is close to the expected behavior after deprecations are removed
(other than that the b->globalref->mod in the printed message here will
be the source module instead of the destination module, which may
sometimes cause confusing printing here, but probably rarely). I also
needed this recently to find a place this warning occurred, so I think
it should be merged now and get feedback later.

Closes #57969

(cherry picked from commit 0682158)
charleskawczynski pushed a commit to charleskawczynski/julia that referenced this issue May 12, 2025
charleskawczynski pushed a commit to charleskawczynski/julia that referenced this issue May 12, 2025
This is close to the expected behavior after deprecations are removed
(other than that the b->globalref->mod in the printed message here will
be the source module instead of the destination module, which may
sometimes cause confusing printing here, but probably rarely). I also
needed this recently to find a place this warning occurred, so I think
it should be merged now and get feedback later.

Closes JuliaLang#57969
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage This should be discussed on a triage call
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants