Add LazyCell::into_inner #106152

SUPERCILEX · 2022-12-26T08:54:25Z

This enables uses cases that need to extract the evaluated value and do something owned with it.

rustbot · 2022-12-26T08:54:32Z

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2022-12-26T08:54:35Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

SUPERCILEX · 2022-12-26T08:54:48Z

@rustbot label +T-libs-api -T-libs

library/core/src/cell/lazy.rs

matklad · 2022-12-26T11:12:26Z

Prior art from the crate:

https://docs.rs/once_cell/1.16.0/src/once_cell/lib.rs.html#713-719

assoc fn to not shadow methods of T
returns result to allow extracting the closure as well

SUPERCILEX · 2022-12-26T21:19:42Z

@matklad Seems reasonable, done.

a1phyr · 2022-12-26T23:04:10Z

For consistency, this should probably be added to LazyLock too

SUPERCILEX · 2022-12-27T00:14:02Z

Sounds reasonable, done.

bors · 2022-12-28T00:56:18Z

☔ The latest upstream changes (presumably #106193) made this pull request unmergeable. Please resolve the merge conflicts.

library/core/src/cell/lazy.rs

library/std/src/sync/lazy_lock.rs

scottmcm · 2023-01-14T01:12:13Z

I don't think I'm a good person to make even a nightly decision about this API, so flipping over to

r? rust-lang/libs-api

WaffleLapkin · 2023-01-19T18:40:49Z

library/core/src/cell/lazy.rs

+    /// assert_eq!(&*lazy, "HELLO, WORLD!");
+    /// assert_eq!(LazyCell::into_inner(lazy).ok(), Some("HELLO, WORLD!".to_string()));
+    /// ```
+    #[unstable(feature = "once_cell", issue = "74465")]


Should this maybe have a separate feature gate? When we'll be stabilizing once cell we'll want to first stabilize some sort of minimal API and only then stabilize other methods...

Suggested change

#[unstable(feature = "once_cell", issue = "74465")]

#[unstable(feature = "once_cell_into_inner", issue = "74465")]

No, it goes the other way around: #105587

That's just because we didn't split originally though?.. It's always easier to stabilize small parts of the API and it's also easier when you don't have to change the gate before stabilizing.

Sure, but why should this method be stabilized separately from the rest of Lazy*? That seems like a decision that can be made when we're actually ready to stabilize the Lazy variants.

That's just generally a good idea to have small feature gates, stabilizing multiple at the same time is a lot simpler and more clear than splitting one into multiple and stabilizing a subset. This method doesn't look essential, so I don't think it is likely it will be included in the first stabilization.

Amanieu · 2023-01-31T14:36:14Z

I find returning a Result here a bit awkward since nowhere else in the LazyCell API do we expose whether a LazyCell has been initialized or not. The version from once_cell does have a get method which exposes this however.

If there was an explicit decision to not expose the uninitialized state then I think we should continue doing this and have LazyCell::into_inner forcibly run the initialization function and return T.

SUPERCILEX · 2023-01-31T23:56:57Z

forcibly run the initialization function

That would make this API mostly useless since the point is to say "if something was initialized, let me do X with it." In my case X is flushing pending tasks from a lazily created thread pool.

I do agree that using a result is super weird. Maybe we should create our own enum LazyState { Initialized, Uninitialized }? Though having a one-off type is a bit of a bummer.

does have a get method which exposes this however.

Maybe lazy should have this too? The crate has it too: https://docs.rs/once_cell/latest/once_cell/sync/struct.Lazy.html#method.get

SUPERCILEX · 2023-04-24T00:18:51Z

Thanks! I added a comment mentioning the feature: #109736 (comment)

tgross35 · 2023-04-24T00:33:53Z

@SUPERCILEX awesome!

Quick note - the gate name lazy_cell_consume might be worth renaming at some point, because Consume memory ordering semantics are a separate thing listed as “future possibilities” for LazyLock/OnceLock. Thats unlikely to ever get implemented though so not a huge deal.

SUPERCILEX · 2023-04-24T00:39:53Z

@tgross35 do you have bors r+ perms? I can rename it to lazy_cell_extra real quick if that's better.

tgross35 · 2023-04-24T05:06:38Z

I do not, but I think it can just be renamed in a separate PR if it's needed

SUPERCILEX · 2023-04-24T05:14:44Z

Sounds good, I'll leave it be then.

Add LazyCell::into_inner This enables uses cases that need to extract the evaluated value and do something owned with it.

bors · 2023-04-24T23:47:36Z

⌛ Testing commit d9256f9 with merge fdeef3e...

bors · 2023-04-25T02:00:22Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing fdeef3e to master...

bors · 2023-04-25T02:00:22Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing fdeef3e to master...

rust-timer · 2023-04-25T04:11:05Z

Finished benchmarking commit (fdeef3e): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.6%	[0.7%, 2.8%]	4
Regressions ❌ (secondary)	6.3%	[5.0%, 8.0%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.9%	[-0.9%, -0.9%]	1
All ❌✅ (primary)	1.6%	[0.7%, 2.8%]	4

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.0%	[1.5%, 2.2%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.0%	[1.5%, 2.2%]	4

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.9%	[2.3%, 5.5%]	6
Improvements ✅ (primary)	-2.9%	[-2.9%, -2.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.9%	[-2.9%, -2.9%]	1

SUPERCILEX · 2023-04-25T05:01:14Z

Wat? I don't see how this could possibly be related. All this PR does is add a method that isn't used anywhere.

Noratrieb · 2023-04-25T05:50:54Z

The usual keccak/cranelift-codegen bimodality, nothing to worry about.

SUPERCILEX · 2023-04-25T05:55:31Z

Whew, thanks!

lqd · 2023-04-25T05:57:57Z

Agreed, @rustbot label: +perf-regression-triaged

rustbot assigned scottmcm Dec 26, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 26, 2022

rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 26, 2022

SUPERCILEX mentioned this pull request Dec 26, 2022

Tracking Issue for once_cell #74465

Closed

9 tasks

This comment has been minimized.

Sign in to view

SUPERCILEX force-pushed the lazycell branch from 9cadb05 to 366e1d4 Compare December 26, 2022 09:07

This comment has been minimized.

Sign in to view

SUPERCILEX force-pushed the lazycell branch from 366e1d4 to d8b0a62 Compare December 26, 2022 09:32

joboet reviewed Dec 26, 2022

View reviewed changes

library/core/src/cell/lazy.rs Outdated Show resolved Hide resolved

SUPERCILEX force-pushed the lazycell branch from d8b0a62 to db0d0a9 Compare December 26, 2022 21:19

SUPERCILEX force-pushed the lazycell branch from db0d0a9 to 6380b1a Compare December 27, 2022 00:13

SUPERCILEX force-pushed the lazycell branch from 6380b1a to 61e7101 Compare December 28, 2022 01:10

yvt reviewed Dec 29, 2022

View reviewed changes

library/core/src/cell/lazy.rs Outdated Show resolved Hide resolved

library/std/src/sync/lazy_lock.rs Outdated Show resolved Hide resolved

SUPERCILEX force-pushed the lazycell branch from 61e7101 to 967b866 Compare December 29, 2022 08:08

rustbot assigned Amanieu and unassigned scottmcm Jan 14, 2023

WaffleLapkin reviewed Jan 19, 2023

View reviewed changes

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 24, 2023

Rollup merge of rust-lang#106152 - SUPERCILEX:lazycell, r=Amanieu

8292aef

Add LazyCell::into_inner This enables uses cases that need to extract the evaluated value and do something owned with it.

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 7 pull requests #110753

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 24, 2023

Rollup merge of rust-lang#106152 - SUPERCILEX:lazycell, r=Amanieu

89833f4

Add LazyCell::into_inner This enables uses cases that need to extract the evaluated value and do something owned with it.

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 6 pull requests #110757

Closed

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 24, 2023

Rollup merge of rust-lang#106152 - SUPERCILEX:lazycell, r=Amanieu

d03ebe4

Add LazyCell::into_inner This enables uses cases that need to extract the evaluated value and do something owned with it.

matthiaskrgr mentioned this pull request Apr 24, 2023

Rollup of 8 pull requests #110772

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 25, 2023

bors merged commit fdeef3e into rust-lang:master Apr 25, 2023

bors mentioned this pull request Apr 25, 2023

Use LazyLock to lazily resolve backtraces #109075

Merged

rustbot added the perf-regression Performance regression. label Apr 25, 2023

rustbot added the perf-regression-triaged The performance regression has been triaged. label Apr 25, 2023

ehuss added this to the 1.71.0 milestone May 30, 2023

behnam-oneschema mentioned this pull request Jan 13, 2024

std: Doc blocking behavior of LazyLock #119870

Merged

tspiteri mentioned this pull request May 28, 2024

Tracking Issue for lazy_cell_into_inner #125623

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LazyCell::into_inner #106152

Add LazyCell::into_inner #106152

SUPERCILEX commented Dec 26, 2022

rustbot commented Dec 26, 2022

rustbot commented Dec 26, 2022

SUPERCILEX commented Dec 26, 2022

This comment has been minimized.

This comment has been minimized.

matklad commented Dec 26, 2022

SUPERCILEX commented Dec 26, 2022

a1phyr commented Dec 26, 2022

SUPERCILEX commented Dec 27, 2022

bors commented Dec 28, 2022

scottmcm commented Jan 14, 2023

WaffleLapkin Jan 19, 2023

SUPERCILEX Jan 19, 2023

WaffleLapkin Jan 20, 2023

SUPERCILEX Jan 20, 2023

WaffleLapkin Feb 1, 2023

Amanieu commented Jan 31, 2023

SUPERCILEX commented Jan 31, 2023

SUPERCILEX commented Apr 24, 2023 •

edited

Loading

tgross35 commented Apr 24, 2023

SUPERCILEX commented Apr 24, 2023

tgross35 commented Apr 24, 2023

SUPERCILEX commented Apr 24, 2023

bors commented Apr 24, 2023

bors commented Apr 25, 2023

bors commented Apr 25, 2023

rust-timer commented Apr 25, 2023

SUPERCILEX commented Apr 25, 2023

Noratrieb commented Apr 25, 2023

SUPERCILEX commented Apr 25, 2023

lqd commented Apr 25, 2023

	#[unstable(feature = "once_cell", issue = "74465")]
	#[unstable(feature = "once_cell_into_inner", issue = "74465")]

Add LazyCell::into_inner #106152

Add LazyCell::into_inner #106152

Conversation

SUPERCILEX commented Dec 26, 2022

rustbot commented Dec 26, 2022

rustbot commented Dec 26, 2022

SUPERCILEX commented Dec 26, 2022

This comment has been minimized.

This comment has been minimized.

matklad commented Dec 26, 2022

SUPERCILEX commented Dec 26, 2022

a1phyr commented Dec 26, 2022

SUPERCILEX commented Dec 27, 2022

bors commented Dec 28, 2022

scottmcm commented Jan 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amanieu commented Jan 31, 2023

SUPERCILEX commented Jan 31, 2023

SUPERCILEX commented Apr 24, 2023 • edited Loading

tgross35 commented Apr 24, 2023

SUPERCILEX commented Apr 24, 2023

tgross35 commented Apr 24, 2023

SUPERCILEX commented Apr 24, 2023

bors commented Apr 24, 2023

bors commented Apr 25, 2023

bors commented Apr 25, 2023

rust-timer commented Apr 25, 2023

Overall result: ❌ regressions - ACTION NEEDED

SUPERCILEX commented Apr 25, 2023

Noratrieb commented Apr 25, 2023

SUPERCILEX commented Apr 25, 2023

lqd commented Apr 25, 2023

SUPERCILEX commented Apr 24, 2023 •

edited

Loading