Take a single root node in range_search #71523
Conversation
The unsafe code can be justified within range_search, as it makes sure to not overlap the returned references, but from the callers perspective it's an entirely safe algorithm and there's no need for the caller to know about the duplication.
rust-highfive
added
the
S-waiting-on-review
|
By the way, I was wondering why it wasn't marked unsafe. Not only could you monkey around by passing in different root, it still returns two mutable handles into the same tree and you can invalidate one handle by changing the tree through the other. Not that I think sprinkling more unsafe words helps readability. What would help readability, I think, is moving the function out of the fat map.rs file into search.rs. Although if you pass a |
| // We duplicate the root NodeRef here -- we will never access it in a way | ||
| // that overlaps references obtained from the root. |
There was a problem hiding this comment.
I don't understand that sentence (but I don't have to). In an emptied tree, range_search will return two references to the same and only edge. Is that no overlap?
|
Yeah, I think the justification here may not be perfect -- but I do think this is the right move. In particular, it seems like the risk of "well you could invalidate the tree" is sort of always true -- we detach the handles from the root node pretty frequently -- and I wouldn't want to add a bunch of unsafe everywhere, for sure not until we have the unsafe-in-unsafe-fns lint. |
|
I personally think that the entirety of the internal btree code should just be unsafe, so I don't feel too strongly about this. @bors r+ rollup |
|
📌 Commit e6cf6a7 has been approved by |
bors
added
S-waiting-on-bors
Rollup of 5 pull requests Successful merges: - rust-lang#71364 (Ignore -Zprofile when building compiler_builtins) - rust-lang#71494 (Fix span of while (let) expressions after lowering) - rust-lang#71517 ( Quick and dirty fix of the unused_braces lint) - rust-lang#71523 (Take a single root node in range_search) - rust-lang#71533 (Revert PR 70566 for const validation fix) Failed merges: r? @ghost
The unsafe code can be justified within range_search, as it makes sure to not
overlap the returned references, but from the callers perspective it's an
entirely safe algorithm and there's no need for the caller to know about the
duplication.
cc @ssomers
r? @Amanieu