Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation about lifetimes to thread::scope. #94763

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Mar 9, 2022

This resolves the last unresolved question of #93203

This was brought up in #94559 (comment)

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 9, 2022
@m-ou-se m-ou-se mentioned this pull request Mar 9, 2022
10 tasks
@m-ou-se m-ou-se force-pushed the scoped-threads-lifetime-docs branch from ff781b7 to 09d0111 Compare March 9, 2022 10:09
@Mark-Simulacrum
Copy link
Member

This looks great -- thank you! Very clear and succinct explanation :)

r=me with commits squashed

@m-ou-se m-ou-se force-pushed the scoped-threads-lifetime-docs branch from c82b279 to 4d56c15 Compare March 9, 2022 14:20
@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 9, 2022

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Mar 9, 2022

📌 Commit 4d56c15 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2022
@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 9, 2022

@bors rollup

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 9, 2022
…s, r=Mark-Simulacrum

Add documentation about lifetimes to thread::scope.

This resolves the last unresolved question of rust-lang#93203

This was brought up in rust-lang#94559 (comment)

r? ``@Mark-Simulacrum``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 9, 2022
…s, r=Mark-Simulacrum

Add documentation about lifetimes to thread::scope.

This resolves the last unresolved question of rust-lang#93203

This was brought up in rust-lang#94559 (comment)

r? ```@Mark-Simulacrum```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 9, 2022
…s, r=Mark-Simulacrum

Add documentation about lifetimes to thread::scope.

This resolves the last unresolved question of rust-lang#93203

This was brought up in rust-lang#94559 (comment)

r? ````@Mark-Simulacrum````
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#91804 (Make some `Clone` impls `const`)
 - rust-lang#92541 (Mention intent of `From` trait in its docs)
 - rust-lang#93057 (Add Iterator::collect_into)
 - rust-lang#94739 (Suggest `if let`/`let_else` for refutable pat in `let`)
 - rust-lang#94754 (Warn users about `||` in let chain expressions)
 - rust-lang#94763 (Add documentation about lifetimes to thread::scope.)
 - rust-lang#94768 (Ignore `close_read_wakes_up` test on SGX platform)
 - rust-lang#94772 (Add miri to the well known conditional compilation names and values)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9e90f8d into rust-lang:master Mar 10, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 10, 2022
@m-ou-se m-ou-se deleted the scoped-threads-lifetime-docs branch March 10, 2022 10:21
Copy link
Contributor

@danielhenrymantilla danielhenrymantilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh damn that was quickly merged! 😅

/// The `'scope` lifetime represents the lifetime of the scope itself.
/// That is: the time during which new scoped threads may be spawned,
/// and also the time during which they might still be running.
/// Once this lifetime ends, all scoped threads are joined.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Once this lifetime ends, all scoped threads are joined.
/// By design, this lifetime only ends once all the scoped threads have been joined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I care about this one because that's the right ordering: 'scope by very definition, outlives the Scope and Packet instances, so the right property is rather "once the scoped threads are all joined, this lifetime may end", hence my suggestion

/// That is: the time during which new scoped threads may be spawned,
/// and also the time during which they might still be running.
/// Once this lifetime ends, all scoped threads are joined.
/// This lifetime starts within the `scope` function, before `f` (the argument to `scope`) starts.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit weird to speak of the beginning of a lifetime, it's not really something that comes up in practice or is important. But I guess it's okay for the sake of explaining stuff:

Suggested change
/// This lifetime starts within the `scope` function, before `f` (the argument to `scope`) starts.
/// This lifetime "starts" within the `scope` function, before `f` (the argument to `scope`) is called.

/// and also the time during which they might still be running.
/// Once this lifetime ends, all scoped threads are joined.
/// This lifetime starts within the `scope` function, before `f` (the argument to `scope`) starts.
/// It ends after `f` returns and all scoped threads have been joined, but before `scope` returns.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// It ends after `f` returns and all scoped threads have been joined, but before `scope` returns.
/// It ends after `f` returns and all scoped threads have been joined, but before `scope` returns[^hrtb].
///
/// [^hrtb]: a property of higher-order (`for<'scope>`) lifetimes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants