Skip to content

doc: nits and fixes for thread API #22689

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

Merged
merged 2 commits into from
Mar 2, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 16 additions & 22 deletions src/libstd/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,25 @@
//! a thread will unwind the stack, running destructors and freeing
//! owned resources. Thread panic is unrecoverable from within
//! the panicking thread (i.e. there is no 'try/catch' in Rust), but
//! panic may optionally be detected from a different thread. If
//! the main thread panics the application will exit with a non-zero
//! the panic may optionally be detected from a different thread. If
//! the main thread panics, the application will exit with a non-zero
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this comma is necessary

Copy link
Member Author

Choose a reason for hiding this comment

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

If the comma is removed, the sentence will have to be slightly re-organised by starting with "The application...". Otherwise, the pause feels very needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexcrichton The comma usage here is correct. See https://owl.english.purdue.edu/owl/resource/607/02/ item 2.

Copy link
Member

Choose a reason for hiding this comment

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

Some people leave it in, some leave it out. I generally put it in. It's fine either way.

//! exit code.
//!
//! When the main thread of a Rust program terminates, the entire program shuts
//! down, even if other threads are still running. However, this module provides
//! convenient facilities for automatically waiting for the termination of a
//! child thread (i.e., join), described below.
//! child thread (i.e., join).
Copy link
Member

Choose a reason for hiding this comment

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

Why lose this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Felt arbitrary and not needed. For consistency, we would need to mention described below for everything that will be described later.

//!
//! ## The `Thread` type
//!
//! Already-running threads are represented via the `Thread` type, which you can
//! Threads are represented via the `Thread` type, which you can
Copy link
Member

Choose a reason for hiding this comment

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

Thread still only represents already-running threads.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed Already-running because it doesn't work well with the line that follows it:

By spawning a new thread, e.g. using the thread::spawn function.

//! get in one of two ways:
//!
//! * By spawning a new thread, e.g. using the `thread::spawn` constructor;
//! * By spawning a new thread, e.g. using the `thread::spawn` function.
//! * By requesting the current thread, using the `thread::current` function.
//!
//! Threads can be named, and provide some built-in support for low-level
//! synchronization described below.
//! synchronization (described below).
//!
//! The `thread::current()` function is available even for threads not spawned
//! by the APIs of this module.
Expand All @@ -59,29 +59,27 @@
//! use std::thread;
//!
//! thread::spawn(move || {
//! println!("Hello, World!");
//! // some computation here
//! // some work here
//! });
//! ```
//!
//! In this example, the spawned thread is "detached" from the current
//! thread, meaning that it can outlive the thread that spawned
//! it. (Note, however, that when the main thread terminates all
//! detached threads are terminated as well.)
//! thread. This means that it can outlive its parent (the thread that spawned
//! it), unless this parent is the main thread.
//!
//! ## Scoped threads
Copy link
Member

Choose a reason for hiding this comment

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

How come this was re-worded?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Having a whole sentence in brackets looks ugly
  • It's good to explain that the thread that spwans another is the parent, (even though it may be obvious)

//!
//! Often a parent thread uses a child thread to perform some particular task,
//! and at some point must wait for the child to complete before continuing.
//! For this scenario, use the `scoped` constructor:
//! For this scenario, use the `thread::scoped` function:
//!
//! ```rust
//! use std::thread;
//!
//! let guard = thread::scoped(move || {
//! println!("Hello, World!");
//! // some computation here
//! // some work here
//! });
//!
//! // do some other work in the meantime
//! let output = guard.join();
//! ```
Expand All @@ -92,11 +90,7 @@
//! terminates) when it is dropped. You can join the child thread in
//! advance by calling the `join` method on the guard, which will also
//! return the result produced by the thread. A handle to the thread
//! itself is available via the `thread` method on the join guard.
//!
//! (Note: eventually, the `scoped` constructor will allow the parent and child
//! threads to data that lives on the parent thread's stack, but some language
//! changes are needed before this is possible.)
//! itself is available via the `thread` method of the join guard.
Copy link
Member

Choose a reason for hiding this comment

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

Why did on switch to of?

Copy link
Member Author

Choose a reason for hiding this comment

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

"on" feels less natural: thread method on the join guard. Is that common usage?

Copy link
Member

Choose a reason for hiding this comment

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

Both are, it's a style thing.

//!
//! ## Configuring threads
//!
Expand All @@ -108,7 +102,7 @@
//! use std::thread;
//!
//! thread::Builder::new().name("child1".to_string()).spawn(move || {
//! println!("Hello, world!")
//! println!("Hello, world!");
//! });
//! ```
//!
Expand All @@ -121,7 +115,7 @@
//! initially not present:
//!
//! * The `thread::park()` function blocks the current thread unless or until
//! the token is available for its thread handle, at which point It atomically
//! the token is available for its thread handle, at which point it atomically
//! consumes the token. It may also return *spuriously*, without consuming the
//! token. `thread::park_timeout()` does the same, but allows specifying a
//! maximum time to block the thread for.
Expand All @@ -143,7 +137,7 @@
//! * It avoids the need to allocate mutexes and condvars when building new
//! synchronization primitives; the threads already provide basic blocking/signaling.
//!
//! * It can be implemented highly efficiently on many platforms.
//! * It can be implemented very efficiently on many platforms.
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

it feels more natural: very efficiently vs. highly efficiently

Copy link
Member

Choose a reason for hiding this comment

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

"hightly efficiently" sounds a bit weird, I do like this better.


#![stable(feature = "rust1", since = "1.0.0")]

Expand Down