Skip to content

Conversation

Keno
Copy link
Member

@Keno Keno commented Oct 3, 2025

Previously, backdated binding warnings were only emitted for constants. This commit extends the backdating mechanism to handle imports and globals. This change was requested by triage to improve compatibility of the new binding partition mechanism given issues like #58511.

With this change the behavior is as follows:

julia> function foo()
           include_string(Main, "a = 42")
           return a
       end

julia> foo()
WARNING: Detected access to binding `Main.a` in a world prior to its definition world.
  Julia 1.12 has introduced more strict world age semantics for global bindings.
  !!! This code may malfunction under Revise.
  !!! This code will error in future versions of Julia.
Hint: Add an appropriate `invokelatest` around the access to this binding.
To make this warning an error, and hence obtain a stack trace, use `julia --depwarn=error`.
42

With julia --depwarn=error, the access is rejected:

julia> foo()
ERROR: UndefVarError: `a` not defined in `Main`

I am still not a huge fan of the backdating mechanism in general, but I think this is the least objectionable of the alternatives and provides a big warning to let people know that something is wrong.

Largely written by Claude.

Previously, backdated binding warnings were only emitted for constants.
This commit extends the backdating mechanism to handle imports and globals.
This change was requested by triage to improve compatibility of the new
binding partition mechanism given issues like #58511.

With this change the behavior is as follows:
```julia
julia> function foo()
           include_string(Main, "a = 42")
           return a
       end

julia> foo()
WARNING: Detected access to binding `Main.a` in a world prior to its definition world.
  Julia 1.12 has introduced more strict world age semantics for global bindings.
  !!! This code may malfunction under Revise.
  !!! This code will error in future versions of Julia.
Hint: Add an appropriate `invokelatest` around the access to this binding.
To make this warning an error, and hence obtain a stack trace, use `julia --depwarn=error`.
42
```

With `julia --depwarn=error`, the access is rejected:

```julia
julia> foo()
ERROR: UndefVarError: `a` not defined in `Main`
```

I am still not a huge fan of the backdating mechanism in general, but
I think this is the least objectionable of the alternatives and provides
a big warning to let people know that something is wrong.
@Keno Keno added the backport 1.12 Change should be backported to release-1.12 label Oct 3, 2025
@Keno Keno changed the title Extend backdating checks for imports and globals Extend backdating checks to imports and globals Oct 3, 2025
@oscardssmith oscardssmith added the needs pkgeval Tests for all registered packages should be run with this change label Oct 3, 2025
@oscardssmith oscardssmith requested a review from vtjnash October 3, 2025 13:42
@KristofferC
Copy link
Member

I pretty much think this is too risky to put in this late in the release process, and that the current status quo is the lesser evil.

For example, this PR fails CI on a random assortment of tests, and Revise stops working:

┌ Error: Failed to revise /tmp/SUk8MQkftz/Order1/src/file1.jl
│   exception =
│    UndefVarError: `Ord1` not defined in `Order1`
│    Suggestion: add an appropriate import or assignment. This global was declared but not assigned.
│    Stacktrace:
│     [1] top-level scope
│       @ /tmp/SUk8MQkftz/Order1/src/file1.jl:1
│    Revise evaluation error at /tmp/SUk8MQkftz/Order1/src/file1.jl:1
│ 
└ @ Revise ~/.julia/packages/Revise/JK6mi/src/packagedef.jl:741

@KristofferC KristofferC mentioned this pull request Oct 6, 2025
44 tasks
vtjnash added a commit that referenced this pull request Oct 6, 2025
Also remove `isdefined` shim, as that feels less logical to me with the
current implementation that uses invokelatest. If the implementation was
changed to use `invokenext`, I think it would make sense to add that back
and also to update `names` to return these also. But invoke next
(similar to backdated constants and #59735) prints an incorrect
suggestion of using invokelatest to get the same behavior. That message
is correct with this current commit state, but wrong relative to the
current implementations that use backdated.
vtjnash added a commit that referenced this pull request Oct 6, 2025
@Keno
Copy link
Member Author

Keno commented Oct 7, 2025

Rebinding test failure is #59613, so we should merge that first.

@Keno
Copy link
Member Author

Keno commented Oct 7, 2025

effects.jl is the combination of a pre-existing codegen model bug (will PR separately) with #59613

@Keno
Copy link
Member Author

Keno commented Oct 7, 2025

will PR separately

#59766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12 needs pkgeval Tests for all registered packages should be run with this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants