Repository navigation
Tracking Issue for option_as_slice #108545
Description
Activity
Updated the discussion in the OP now that the implementation relies on the layout estimate only for performance, not for soundness.
Stabilization Report
Implementation History
- The initial implementation was in Add
Option::as_(mut_)slice#105871 - Move
Option::as_sliceto an always-sound implementation #108623 fixed a potential soundness issue - move Option::as_slice to intrinsic #109179 moved the implementation to an intrinsic
- (Future) There may be a future PR which removes the intrinsic in favor of
offset_of!once that is available for enums
API summary
The API is summarized at the top of this tracking issue.
Experience Report
Shortly after the initial implementation, #108678 dogfooded the feature in the compiler, adding 8 lines and removing 17. All four of the cases were both perf and legibility wins. The author notes that there he found some other cases outside of rustc where the feature could be applied, though isn't yet because of not being stabilized, even though his search was merely for a naive implementation of the feature, there may be cases where people have resorted to using a
Vecor other type instead of anOptionto getas_slice. Those cases aren't as easy to find though.Stabilizing this feature will bring the standard library up to parity with the author's optional crate (though that is only for bools, ints and floats).
The API as it is now mirrors that of other collections (notably array,
Vec,BinaryHeapandVecDequeas well as various of their iterators. The only possible extension that was hinted at during development was toDerefOptioninto a possibly empty slice, but this wasn't attempted due to being a breaking change.Finally, I want to note that we can still change the implementation once
offset_of!on enums is available, as the API surface will stay the same. So this seems like as good a time as any to stabilize this feature. Shall we?ping @rust-lang/libs-api
- The initial implementation was in Add
@rfcbot merge
Reacted by llogiqReacted by Johannes Dahlström, scottmcm and CosminPerRamTeam member @BurntSushi has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
- added and removed
on Sep 17, 2023 🔔 This is now entering its final comment period, as per the review above. 🔔
Is it a hard requirement for an empty slice to point to the same place as if
OptionwasSome? Because the pointer part of slice just need to be non-null and sufficiently aligned, so using the address of anOptionitself (as well asNonNull::dangling().as_ptr()) is always the sound approach.Is it a hard requirement for an empty slice to point to the same place as if
OptionwasSome?Well, the API doesn't guarantee it in the docs, so I don't think it's a hard requirement.
It's an important QoI detail for performance, though. By returning the empty slice pointing at where the item would be, it's always returning the same pointer. That simplifies codegen and optimizations, by not needing the extra conditional instruction.
Compare these: https://rust.godbolt.org/z/reqcM84xz
Reacted by llogiq and Martin KröningReacted by Andrew Gallant and Martin KröningAs there were no concerns brought up so far, I'll likely push a stabilization PR later today.
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
- added a commit that references this issue
on Oct 5, 2023 This should be closed as the feature was stabilized.
Reacted by llogiq
Feature gate:
#![feature(option_as_slice)]This is a tracking issue for the
Option::as_sliceandOption::as_mut_slicemethods.The functions return an immutable or mutable slice to the value contained in the Option, if any. Otherwise an empty slice is returned.
Public API
Steps / History
Option::as_(mut_)slice#105871Option::as_sliceto an always-sound implementation #108623Option::as_(mut_)slice#116220Unresolved Questions
The current implementation contains an optimization which relies on the fact that layout randomization as currently implemented only applies to individual variants, never to the discriminant, and thus the offset of thevaluewithinSome(value)is always either 0 (because of niche optimization) or would be the same if the type wasOption<MaybeUninit<T>>instead ofOption<T>.Before stabilization, the implementation should be changed to either use an intrinsic to get the offset or extend theoffset_of!macro (recently merged RFC#3308) to cover enum variants and use that. There might also be a smaller solution (that we'd still need to check re the rules of Rust layout), which would simply bemem::size_of::<Option<T>>() - mem::size_of::<T>(). I can see that this works for all types I've tested it with, and indeed I'm quite sure that it will work perfectly with the current layout implementation, but I'd like someone from the types team have a look at it before actually using it in a stable API.The implementation now uses an approach that's always sound (just less efficient than optimal if the hack guesses wrong). A stabilization conversation would probably still want to discuss whether we're comfortable with shipping that approach as stable (since the implementation could be improved non-breakingly later) or we'd prefer to wait for a more principled approach (such as an extension of
offset_of!from rust-lang/rfcs#3308 that allows enum variants) first.Footnotes
https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html ↩