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.

Sign up for GitHub

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

Jump to bottom

Fix mutex's docs inconsistency #40608

Merged
merged 1 commit into from
Apr 5, 2017
Merged

Fix mutex's docs inconsistency #40608

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions src/libstd/sync/mutex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use sys_common::poison::{self, TryLockError, TryLockResult, LockResult};
///
/// The mutexes in this module implement a strategy called "poisoning" where a
/// mutex is considered poisoned whenever a thread panics while holding the
/// lock. Once a mutex is poisoned, all other threads are unable to access the
/// mutex. Once a mutex is poisoned, all other threads are unable to access the
/// data by default as it is likely tainted (some invariant is not being
/// upheld).
///
Expand Down Expand Up @@ -115,7 +115,7 @@ pub struct Mutex<T: ?Sized> {
// Note that this mutex is in a *box*, not inlined into the struct itself.
// Once a native mutex has been used once, its address can never change (it
// can't be moved). This mutex type can be safely moved at any time, so to
// ensure that the native mutex is used correctly we box the inner lock to
// ensure that the native mutex is used correctly we box the inner mutex to
// give it a constant address.
inner: Box<sys::Mutex>,
poison: poison::Flag,
Expand Down Expand Up @@ -183,7 +183,7 @@ impl<T: ?Sized> Mutex<T> {
/// Acquires a mutex, blocking the current thread until it is able to do so.
Copy link
Contributor

Choose a reason for hiding this comment

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

Acquires a mutex, blocking

Two lines after this, the comment speaks of acquiring "the lock," should this instance be modified as well?

Copy link

Choose a reason for hiding this comment

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

Concern here: "lock" is generic, while "mutex" is shorthand for "mutually exclusive lock" and therefore seems most specific and correct to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same paragraph directly quoted in #40176, would you say it would have been better to change the one instance of "the lock" to "the mutex" instead of the other way around?

Copy link

Choose a reason for hiding this comment

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

Yes "lock" or "acquire" as the verb, and "mutex" as the noun. Therefore "the mutex" makes more sense than "the lock". 😃

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 agree with @ScottAbbey on this one. For non-english speaker, acquiring a lock better than acquiring a mutex which doesn't mean much.

Copy link

Choose a reason for hiding this comment

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

@GuillaumeGomez while I agree, this isn't just pros. We're discussing technical documentation and as such should be as specific as possible, without being exclusionary.

If we're going with the general term, then we need to the MSDN thing of explaining that the lock is exclusive and will block any threads that attempt to acquire the lock while the lock is held.

IMO this gets verbose, but does have the advantage of being very approachable and lends itself well to education as well as to translation.

///
/// This function will block the local thread until it is available to acquire
/// the mutex. Upon returning, the thread is the only thread with the mutex
/// the mutex. Upon returning, the thread is the only thread with the lock
/// held. An RAII guard is returned to allow scoped unlock of the lock. When
/// the guard goes out of scope, the mutex will be unlocked.
///
Expand Down Expand Up @@ -267,9 +267,9 @@ impl<T: ?Sized> Mutex<T> {
}
}

/// Determines whether the lock is poisoned.
/// Determines whether the mutex is poisoned.
///
/// If another thread is active, the lock can still become poisoned at any
/// If another thread is active, the mutex can still become poisoned at any
/// time. You should not trust a `false` value for program correctness
/// without additional synchronization.
///
Expand Down Expand Up @@ -312,7 +312,7 @@ impl<T: ?Sized> Mutex<T> {
#[stable(feature = "mutex_into_inner", since = "1.6.0")]
pub fn into_inner(self) -> LockResult<T> where T: Sized {
// We know statically that there are no outstanding references to
// `self` so there's no need to lock the inner lock.
// `self` so there's no need to lock the inner mutex.
//
// To get the inner value, we'd like to call `data.into_inner()`,
// but because `Mutex` impl-s `Drop`, we can't move out of it, so
Expand Down Expand Up @@ -353,7 +353,7 @@ impl<T: ?Sized> Mutex<T> {
#[stable(feature = "mutex_get_mut", since = "1.6.0")]
pub fn get_mut(&mut self) -> LockResult<&mut T> {
// We know statically that there are no other references to `self`, so
// there's no need to lock the inner lock.
// there's no need to lock the inner mutex.
let data = unsafe { &mut *self.data.get() };
poison::map_result(self.poison.borrow(), |_| data )
}
Expand Down