Skip to content

Improve tree_arena's safety documentation#1755

Open
PoignardAzur wants to merge 4 commits into
linebender:mainfrom
PoignardAzur:audit_tree_arena_2
Open

Improve tree_arena's safety documentation#1755
PoignardAzur wants to merge 4 commits into
linebender:mainfrom
PoignardAzur:audit_tree_arena_2

Conversation

@PoignardAzur

@PoignardAzur PoignardAzur commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

See #masonry > Refactoring tree arena for disjoint borrows for rationale.

Note that this PR neither changes the behavior of tree_arena_unsafe nor the conditions under which it's sound.

The PR makes find_mut_inner unsafe, but I'd argue it was already unsafe in the sense that calling it with the wrong arguments would lead to UB. This was acceptable because the method was private and the rest of the code never called it with wrong arguments, but I would argue that marking the method as unsafe is more accurate and helpful.

Similarly, the PR points out the box invalidation problem, but that problem already exists in our current implementation.

@PoignardAzur PoignardAzur changed the title Audit tree arena 2 Improve tree_arena's safety documentation Apr 18, 2026
@PoignardAzur PoignardAzur marked this pull request as ready for review April 19, 2026 16:35
Comment on lines +418 to +421
// This is sound because `self.arena` is a `&DataMap<T>`, and the only other code
// path that can access `node_cell` is `ArenaMutList::find_mut_inner`.
// `ArenaMutList::parent_arena` is a `&mut DataMap<T>`, which means that an
// `ArenaRefList` can't be constructed as long as an `ArenaMutList` is live.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Come to think of it, this is already inaccurate because of ArenaMutList::reborrow.

Uuuugh.

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.

1 participant