Skip to content

Preserve creation world age in binding partition lists#61200

Open
timholy wants to merge 2 commits intomasterfrom
teh/bpart_guard_decls
Open

Preserve creation world age in binding partition lists#61200
timholy wants to merge 2 commits intomasterfrom
teh/bpart_guard_decls

Conversation

@timholy
Copy link
Member

@timholy timholy commented Feb 28, 2026

This has two commits which should perhaps be merged without squashing. The first commit fixes two cases in which master may backdate a binding to before the world age of the corresponding module. The second commit introduces a new kind of guard partition, PARTITION_KIND_DECLARED_GUARD, whose purpose is to record the world age of creation for bindings that do not yet have an assigned value (e.g., export x or public y).

This should resolve the challenges facing #60736 but is kept independent of that work. I am uneasy about declaring this backport-worthy, at least for 1.12. Soliciting the thoughts of others on this point, as #60736 will not be backportable without this.

Co-written with Claude.

timholy and others added 2 commits February 28, 2026 05:33
Two related fixes to ensure that the min_world of a GUARD or FAILED
partition is never silently backdated past the world at which the
binding's module was first established.

Previously, jl_implicit_import_resolved would merge a newly-cached
guard partition backward into the adjacent (newer) partition when both
had the same kind and restriction. For non-guard kinds (IMPLICIT_CONST,
IMPLICIT_GLOBAL) this merging is correct and efficient. For GUARD and
FAILED, it erased the semantically meaningful world boundary encoded by
the implicit resolution: for a module M with `using Core` established at
world `w`, the resolution already returns `min_world = w`, but a
subsequent query at any world < `w` caused that boundary to be erased,
extending the GUARD partition all the way back to `min_world = 0`.

The backdate loop in `jl_declare_constant_val3` compounded this: it
called `jl_get_binding_partition` (which triggers implicit resolution) to
walk backward through partition history, inadvertently manufacturing a
pre-creation GUARD[0, `w`-1] partition that got merged away the `w`
boundary, and also caused BACKDATED_CONST to cover worlds where the
module did not yet exist.

The fix:
1. Skip the backward merge in `jl_implicit_import_resolved` when the
   partition kind is GUARD or FAILED.
2. Use `jl_get_binding_partition_if_present` in the backdate loop so it
   only traverses partitions that already exist, stopping at the oldest
   one without creating new ones via implicit resolution.

After these changes, a GUARD partition's min_world reliably reflects the
world at which the binding's module (and its using graph) was first
established.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This adds a new binding partition kind `PARTITION_KIND_DECLARED_GUARD` to
distinguish bindings that were explicitly declared via `public x` or `export x`
on an undefined name from purely implicit guard partitions created by lookup
misses.

Previously, `export foo` on an undefined name would set the EXPORTED flag on
the existing GUARD partition. With this change, it creates a new DECLARED_GUARD
partition, giving the explicit declaration its own world-stamped record. This
allows tooling and error messages to distinguish "someone explicitly declared
this name" from "this name was simply never resolved."

This will be useful in resolving the challenges currently facing #60736.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Keno
Copy link
Member

Keno commented Mar 2, 2026

The first change is semantically problematic. The only semantic information that the world bounds carry is whether or not the content they describe is valid for that particular set of world ages. We cannot make any guarantees that a particular minimum world implies something specific. Moreover, not backdating before the module creation defeats the purpose of the backdating (e.g. in an (@eval module Foo; const x = 1; end). x). Of course backdating itself is annoying for this reason, but the existing semantics are intentional on this point.

As for the second change, I'm not sure I understand the motivation EXPORTED | GUARD should already have these semantics. Can you explain what you're trying to accomplish?

@timholy
Copy link
Member Author

timholy commented Mar 2, 2026

#60736 (comment)

The bigger picture is the following: currently, Revise has to scan every type in the system to get a comprehensive understanding of dependencies because the type "backedges" are incomplete. Without introducing callbacks into Julia's internal type-creation functions, it's currently too slow to do this at each REPL prompt (xref timholy/Revise.jl#1014). Thus users are likely to have to wait for this to happen when they modify types. Even with recent very large speedups (timholy/Revise.jl#1013 (comment)), this can take ~20s for some users. Much of the time is spent on names, so if we could limit to just "names with changes since the last world age" it would speed things greatly. But at present that seems hard because if someone adds export foo to a module without defining it, there is only a single guard entry created for foo and it gets backdated to the module creation age.

FYI the next release of Revise will likely make type-revision opt-in unless this performance issue gets fixed.

@Keno
Copy link
Member

Keno commented Mar 2, 2026

But at present that seems hard because if someone adds export foo to a module without defining it, there is only a single guard entry created for foo and it gets backdated to the module creation age.

That's not correct. The comment you linked is talking about public, which is a module property, because it is advisory and not semantic. Flags like export that affect resolution are partitioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants