From 2e71187d97cb323a2c3447ae5df07c474960e0d3 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Wed, 5 Nov 2025 18:49:44 -0800 Subject: [PATCH] In `Option::get_or_insert_with()`, forget the `None` instead of dropping it. This allows eliminating the `T: [const] Destruct` bounds and avoids generating an implicit `drop_in_place::>()` that will never do anything. Ideally, the compiler would prove that that drop is not necessary itself, but it currently doesn't, even with `const_precise_live_drops` enabled. --- library/core/src/option.rs | 17 ++++++++++++++--- library/coretests/tests/option.rs | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index e3c4758bc6af5..2a7eea056652a 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -1776,7 +1776,7 @@ impl Option { #[rustc_const_unstable(feature = "const_option_ops", issue = "143956")] pub const fn get_or_insert_default(&mut self) -> &mut T where - T: [const] Default + [const] Destruct, + T: [const] Default, { self.get_or_insert_with(T::default) } @@ -1804,10 +1804,21 @@ impl Option { pub const fn get_or_insert_with(&mut self, f: F) -> &mut T where F: [const] FnOnce() -> T + [const] Destruct, - T: [const] Destruct, { if let None = self { - *self = Some(f()); + // The effect of this is identical to + // *self = Some(f()); + // except that it does not drop the old value of `*self`. This is not a leak, because + // we just checked that the old value is `None`, which contains no fields to drop. + // + // This implementation avoids needing a `T: [const] Destruct` bound and avoids + // possibly compiling needless drop code. Ideally, the compiler would prove that that + // drop is not necessary itself, but it currently doesn't, even with + // `const_precise_live_drops` enabled. + // + // It could also be expressed as `unsafe { core::ptr::write(self, Some(f())) }`, but + // no reason is currently known to use additional unsafe code here. + mem::forget(mem::replace(self, Some(f()))); } // SAFETY: a `None` variant for `self` would have been replaced by a `Some` diff --git a/library/coretests/tests/option.rs b/library/coretests/tests/option.rs index fc0f82ad6bb38..3df7afb4f3bea 100644 --- a/library/coretests/tests/option.rs +++ b/library/coretests/tests/option.rs @@ -495,6 +495,30 @@ const fn option_const_mut() { */ } +/// Test that `Option::get_or_insert_default` is usable in const contexts, including with types that +/// do not satisfy `T: const Destruct`. +#[test] +fn const_get_or_insert_default() { + const OPT_DEFAULT: Option> = { + let mut x = None; + x.get_or_insert_default(); + x + }; + assert!(OPT_DEFAULT.is_some()); +} + +/// Test that `Option::get_or_insert_with` is usable in const contexts, including with types that +/// do not satisfy `T: const Destruct`. +#[test] +fn const_get_or_insert_with() { + const OPT_WITH: Option> = { + let mut x = None; + x.get_or_insert_with(Vec::new); + x + }; + assert!(OPT_WITH.is_some()); +} + #[test] fn test_unwrap_drop() { struct Dtor<'a> {