Skip to content

Commit e7bce32

Browse files
committed
Auto merge of #148562 - kpreid:get-init-drop, r=oli-obk
In `Option::get_or_insert_with()`, forget the `None` instead of dropping it. Per #148486 (comment) In `Option::get_or_insert_with()`, after replacing the `None` with `Some`, forget the `None` instead of dropping it. This allows eliminating the `T: [const] Destruct` bounds, making the functions more flexible in (unstable) const contexts, and avoids generating an implicit `drop_in_place::<Option<T>>()` that will never do anything (and which might even persist after optimization).
2 parents 2d76d9b + c1b51dd commit e7bce32

2 files changed

Lines changed: 42 additions & 3 deletions

File tree

library/core/src/option.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1777,7 +1777,7 @@ impl<T> Option<T> {
17771777
#[rustc_const_unstable(feature = "const_option_ops", issue = "143956")]
17781778
pub const fn get_or_insert_default(&mut self) -> &mut T
17791779
where
1780-
T: [const] Default + [const] Destruct,
1780+
T: [const] Default,
17811781
{
17821782
self.get_or_insert_with(T::default)
17831783
}
@@ -1805,10 +1805,25 @@ impl<T> Option<T> {
18051805
pub const fn get_or_insert_with<F>(&mut self, f: F) -> &mut T
18061806
where
18071807
F: [const] FnOnce() -> T + [const] Destruct,
1808-
T: [const] Destruct,
18091808
{
18101809
if let None = self {
1811-
*self = Some(f());
1810+
// The effect of the following statement is identical to
1811+
// *self = Some(f());
1812+
// except that it does not drop the old value of `*self`. This is not a leak, because
1813+
// we just checked that the old value is `None`, which contains no fields to drop.
1814+
// This implementation strategy
1815+
//
1816+
// * avoids needing a `T: [const] Destruct` bound, to the benefit of `const` callers,
1817+
// * and avoids possibly compiling needless drop code (as would sometimes happen in the
1818+
// previous implementation), to the benefit of non-`const` callers.
1819+
//
1820+
// FIXME(const-hack): It would be nice if this weird trick were made obsolete
1821+
// (though that is likely to be hard/wontfix).
1822+
//
1823+
// It could also be expressed as `unsafe { core::ptr::write(self, Some(f())) }`, but
1824+
// no reason is currently known to use additional unsafe code here.
1825+
1826+
mem::forget(mem::replace(self, Some(f())));
18121827
}
18131828

18141829
// SAFETY: a `None` variant for `self` would have been replaced by a `Some`

library/coretests/tests/option.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,30 @@ const fn option_const_mut() {
495495
*/
496496
}
497497

498+
/// Test that `Option::get_or_insert_default` is usable in const contexts, including with types that
499+
/// do not satisfy `T: const Destruct`.
500+
#[test]
501+
fn const_get_or_insert_default() {
502+
const OPT_DEFAULT: Option<Vec<bool>> = {
503+
let mut x = None;
504+
x.get_or_insert_default();
505+
x
506+
};
507+
assert!(OPT_DEFAULT.is_some());
508+
}
509+
510+
/// Test that `Option::get_or_insert_with` is usable in const contexts, including with types that
511+
/// do not satisfy `T: const Destruct`.
512+
#[test]
513+
fn const_get_or_insert_with() {
514+
const OPT_WITH: Option<Vec<bool>> = {
515+
let mut x = None;
516+
x.get_or_insert_with(Vec::new);
517+
x
518+
};
519+
assert!(OPT_WITH.is_some());
520+
}
521+
498522
#[test]
499523
fn test_unwrap_drop() {
500524
struct Dtor<'a> {

0 commit comments

Comments
 (0)