Skip to content

bpart: When backdating replace the entire bpart chain #57341

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

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Conversation

Keno
Copy link
Member

@Keno Keno commented Feb 10, 2025

Rather than rewriting the restriction of the bparts. With this, I believe this removes that last point where restriction is overwritten after construction, hopefully allowing us to go back to the original design where restriction is const after construction.

@vtjnash
Copy link
Member

vtjnash commented Feb 12, 2025

SGTM, though ASAN says you're missing a GC root in this function for the object allocated

Rather than rewriting the `restriction` of the bparts. With this, I
believe this removes that last point where `restriction` is overwritten
after construction, hopefully allowing us to go back to the original
design where `restriction` is `const` after construction.
@Keno Keno merged commit 90046a0 into master Feb 12, 2025
4 checks passed
@Keno Keno deleted the kf/backdatetweak branch February 12, 2025 23:01
Keno added a commit that referenced this pull request Feb 13, 2025
The original design for the BindingPartition datastructure had ->restriction
and ->kind as separate non-atomic fields. However, to support the old semantics,
we created an intermediate state where both the restriciton and the kind were
placed into ->restriction as a pointer-int-union (i.e. using the low three
bits of the pointer to store an int). In #57341, I removed that last semantic
place that needed to update these both atomically. This PR removes all the
remaining non-semantic places and changes the datastructure back to its
indended design. This is a necessary prerequisitve to be able to use more
than three ->kind bits, which will be required for export invalidation
(#57377), as well as some nicer error messages in failure cases.
vtjnash pushed a commit that referenced this pull request Feb 13, 2025
The original design for the BindingPartition datastructure had
->restriction and ->kind as separate non-atomic fields. However, to
support the old semantics, we created an intermediate state where both
the restriciton and the kind were placed into ->restriction as a
pointer-int-union (i.e. using the low three bits of the pointer to store
an int). In #57341, I removed that last semantic place that needed to
update these both atomically. This PR removes all the remaining
non-semantic places and changes the datastructure back to its indended
design. This is a necessary prerequisitve to be able to use more than
three ->kind bits, which will be required for export invalidation
(#57377), as well as some nicer error messages in failure cases.
@KristofferC KristofferC added the backport 1.12 Change should be backported to release-1.12 label Feb 14, 2025
KristofferC pushed a commit that referenced this pull request Feb 14, 2025
Rather than rewriting the `restriction` of the bparts. With this, I
believe this removes that last point where `restriction` is overwritten
after construction, hopefully allowing us to go back to the original
design where `restriction` is `const` after construction.

(cherry picked from commit 90046a0)
KristofferC pushed a commit that referenced this pull request Feb 14, 2025
The original design for the BindingPartition datastructure had
->restriction and ->kind as separate non-atomic fields. However, to
support the old semantics, we created an intermediate state where both
the restriciton and the kind were placed into ->restriction as a
pointer-int-union (i.e. using the low three bits of the pointer to store
an int). In #57341, I removed that last semantic place that needed to
update these both atomically. This PR removes all the remaining
non-semantic places and changes the datastructure back to its indended
design. This is a necessary prerequisitve to be able to use more than
three ->kind bits, which will be required for export invalidation
(#57377), as well as some nicer error messages in failure cases.

(cherry picked from commit 40fbc88)
@KristofferC KristofferC mentioned this pull request Feb 14, 2025
31 tasks
KristofferC added a commit that referenced this pull request Feb 17, 2025
Backported PRs:
- [x] #57346 <!-- lowering: Only try to define the method once -->
- [x] #57341 <!-- bpart: When backdating replace the entire bpart chain
-->
- [x] #57381 <!-- staticdata: Set min validation world to require world
-->
- [x] #57357 <!-- Only implicitly `using` Base, not Core -->
- [x] #57383 <!-- staticdata: Fix typo in recursive edge revalidation
-->
- [x] #57385 <!-- bpart: Move kind enum into its intended place -->
- [x] #57275 <!-- Compiler: fix unsoundness of getfield_tfunc on Tuple
Types -->
- [x] #57378 <!-- print admonition for auto-import only once per module
-->
- [x] #57392 <!-- [LateLowerGCFrame] fix PlaceGCFrameReset for
returns_twice -->
- [x] #57388 <!-- Bump JuliaSyntax to v1.0.2 -->
- [x] #57266 <!-- 🤖 [master] Bump the Statistics stdlib from d49c2bf to
77bd570 -->
- [x] #57395 <!-- lowering: fix has_fcall computation -->
- [x] #57204 <!-- Clarify mathematical definition of `gcd` -->
- [x] #56794 <!-- Make `Pairs` public -->
- [x] #57407 <!-- staticdata: corrected implementation of
jl_collect_new_roots -->
- [x] #57405 <!-- bpart: Also partition the export flag -->
- [x] #57420 <!-- Compiler: Fix check for IRShow definedness -->
- [x] #55875 <!-- fix `(-Inf)^-1` inconsistency -->
- [x] #57317 <!-- internals: add _defaultctor function for defining
ctors -->
- [x] #57406 <!-- bpart: Ignore guard bindings for ambiguity purposes
-->
- [x] #49933 <!-- Allow for :foreigncall to transition to GC safe
automatically -->
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Feb 17, 2025
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.

3 participants