From f1c1fc2dbe442bedf214219d281b0d83b42cff67 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Wed, 14 Feb 2018 09:19:01 +0200 Subject: [PATCH 01/10] rephrase UnsafeCell doc Make UnsafeCell doc easier to follow --- src/libcore/cell.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index ec0d1b704dceb..6270e87c9cce9 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1164,11 +1164,12 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// The compiler makes optimizations based on the knowledge that `&T` is not mutably aliased or /// mutated, and that `&mut T` is unique. When building abstractions like `Cell`, `RefCell`, /// `Mutex`, etc, you need to turn these optimizations off. `UnsafeCell` is the only legal way -/// to do this. When `UnsafeCell` is immutably aliased, it is still safe to obtain a mutable -/// reference to its interior and/or to mutate it. However, it is up to the abstraction designer -/// to ensure that no two mutable references obtained this way are active at the same time, and -/// that there are no active mutable references or mutations when an immutable reference is obtained -/// from the cell. This is often done via runtime checks. +/// to do this. When `UnsafeCell` _itself_ is immutably aliased, it is still safe to obtain +/// a mutable reference to its _interior_ and/or to mutate the interior. However, it is up to +/// the abstraction designer to ensure that no two mutable references obtained this way are active +/// at the same time, there are no active immutable reference when a mutable reference is obtained +/// from the cell, and that there are no active mutable references or mutations when an immutable +/// reference is obtained. This is often done via runtime checks. /// /// Note that while mutating or mutably aliasing the contents of an `& UnsafeCell` is /// okay (provided you enforce the invariants some other way); it is still undefined behavior @@ -1240,9 +1241,9 @@ impl UnsafeCell { /// Gets a mutable pointer to the wrapped value. /// /// This can be cast to a pointer of any kind. - /// Ensure that the access is unique when casting to - /// `&mut T`, and ensure that there are no mutations or mutable - /// aliases going on when casting to `&T` + /// Ensure that the access is unique (no active references, mutable or not) + /// when casting to `&mut T`, and ensure that there are no mutations + /// or mutable aliases going on when casting to `&T` /// /// # Examples /// From b9b82490206fc41eb10662d6847a1ad28618ee59 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Wed, 14 Feb 2018 10:11:37 +0200 Subject: [PATCH 02/10] fix tidy checks --- src/libcore/cell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 6270e87c9cce9..6f91e743999b1 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1165,7 +1165,7 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// mutated, and that `&mut T` is unique. When building abstractions like `Cell`, `RefCell`, /// `Mutex`, etc, you need to turn these optimizations off. `UnsafeCell` is the only legal way /// to do this. When `UnsafeCell` _itself_ is immutably aliased, it is still safe to obtain -/// a mutable reference to its _interior_ and/or to mutate the interior. However, it is up to +/// a mutable reference to its _interior_ and/or to mutate the interior. However, it is up to /// the abstraction designer to ensure that no two mutable references obtained this way are active /// at the same time, there are no active immutable reference when a mutable reference is obtained /// from the cell, and that there are no active mutable references or mutations when an immutable @@ -1243,7 +1243,7 @@ impl UnsafeCell { /// This can be cast to a pointer of any kind. /// Ensure that the access is unique (no active references, mutable or not) /// when casting to `&mut T`, and ensure that there are no mutations - /// or mutable aliases going on when casting to `&T` + /// or mutable aliases going on when casting to `&T` /// /// # Examples /// From 273166e7655dc736f444de43505c3ddda5c742f7 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Tue, 27 Feb 2018 16:41:28 +0200 Subject: [PATCH 03/10] remove italic remove italic as per @GuillaumeGomez suggestion --- src/libcore/cell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 6f91e743999b1..4a1d68efc99b0 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1164,8 +1164,8 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// The compiler makes optimizations based on the knowledge that `&T` is not mutably aliased or /// mutated, and that `&mut T` is unique. When building abstractions like `Cell`, `RefCell`, /// `Mutex`, etc, you need to turn these optimizations off. `UnsafeCell` is the only legal way -/// to do this. When `UnsafeCell` _itself_ is immutably aliased, it is still safe to obtain -/// a mutable reference to its _interior_ and/or to mutate the interior. However, it is up to +/// to do this. When `UnsafeCell` itself is immutably aliased, it is still safe to obtain +/// a mutable reference to its interior and/or to mutate the interior. However, it is up to /// the abstraction designer to ensure that no two mutable references obtained this way are active /// at the same time, there are no active immutable reference when a mutable reference is obtained /// from the cell, and that there are no active mutable references or mutations when an immutable From d9b8724a8072d51c014d2d8c6852e5a398c757d3 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Tue, 27 Feb 2018 23:21:04 +0200 Subject: [PATCH 04/10] style fix --- src/libcore/cell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 4a1d68efc99b0..01e618421cc53 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1172,7 +1172,7 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// reference is obtained. This is often done via runtime checks. /// /// Note that while mutating or mutably aliasing the contents of an `& UnsafeCell` is -/// okay (provided you enforce the invariants some other way); it is still undefined behavior +/// okay (provided you enforce the invariants some other way), it is still undefined behavior /// to have multiple `&mut UnsafeCell` aliases. /// /// From 50f5ea919231b41b7d8ccb1023b4dc0fd6e6730d Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Tue, 27 Feb 2018 23:23:19 +0200 Subject: [PATCH 05/10] Simplify Merge three rules into one following @cramertj --- src/libcore/cell.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 01e618421cc53..b57667df99169 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1165,11 +1165,15 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// mutated, and that `&mut T` is unique. When building abstractions like `Cell`, `RefCell`, /// `Mutex`, etc, you need to turn these optimizations off. `UnsafeCell` is the only legal way /// to do this. When `UnsafeCell` itself is immutably aliased, it is still safe to obtain -/// a mutable reference to its interior and/or to mutate the interior. However, it is up to -/// the abstraction designer to ensure that no two mutable references obtained this way are active -/// at the same time, there are no active immutable reference when a mutable reference is obtained -/// from the cell, and that there are no active mutable references or mutations when an immutable -/// reference is obtained. This is often done via runtime checks. +/// a mutable reference to its interior and/or to mutate the interior. However, the abstraction +/// designer must ensure that any active mutable references to the interior obtained this way does +/// not co-exist with other active references to the interior, either mutable or not. This is often +/// done via runtime checks. Naturally, several active immutable references to the interior can +/// co-exits with each other (but not with a mutable reference). +/// +/// To put it in other words, if a mutable reference to the contents is active, no other references +/// can be active at the same time, and if an immutable reference to the contents is active, then +/// only other immutable reference may be active. /// /// Note that while mutating or mutably aliasing the contents of an `& UnsafeCell` is /// okay (provided you enforce the invariants some other way), it is still undefined behavior From ff6754c68ec293d46eb294f3b7efeca8300b2251 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Tue, 27 Feb 2018 23:34:38 +0200 Subject: [PATCH 06/10] fix tidy checks --- src/libcore/cell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index b57667df99169..6f18b8466a665 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1166,12 +1166,12 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// `Mutex`, etc, you need to turn these optimizations off. `UnsafeCell` is the only legal way /// to do this. When `UnsafeCell` itself is immutably aliased, it is still safe to obtain /// a mutable reference to its interior and/or to mutate the interior. However, the abstraction -/// designer must ensure that any active mutable references to the interior obtained this way does -/// not co-exist with other active references to the interior, either mutable or not. This is often +/// designer must ensure that any active mutable references to the interior obtained this way does +/// not co-exist with other active references to the interior, either mutable or not. This is often /// done via runtime checks. Naturally, several active immutable references to the interior can /// co-exits with each other (but not with a mutable reference). /// -/// To put it in other words, if a mutable reference to the contents is active, no other references +/// To put it in other words, if a mutable reference to the contents is active, no other references /// can be active at the same time, and if an immutable reference to the contents is active, then /// only other immutable reference may be active. /// From 78789add6c4ab8a4b81e717803be63c384438595 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Tue, 27 Feb 2018 23:52:47 +0200 Subject: [PATCH 07/10] and some more tidy checks --- src/libcore/cell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 6f18b8466a665..8accbc204c1d7 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1168,12 +1168,12 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// a mutable reference to its interior and/or to mutate the interior. However, the abstraction /// designer must ensure that any active mutable references to the interior obtained this way does /// not co-exist with other active references to the interior, either mutable or not. This is often -/// done via runtime checks. Naturally, several active immutable references to the interior can +/// done via runtime checks. Naturally, several active immutable references to the interior can /// co-exits with each other (but not with a mutable reference). /// /// To put it in other words, if a mutable reference to the contents is active, no other references /// can be active at the same time, and if an immutable reference to the contents is active, then -/// only other immutable reference may be active. +/// only other immutable reference may be active. /// /// Note that while mutating or mutably aliasing the contents of an `& UnsafeCell` is /// okay (provided you enforce the invariants some other way), it is still undefined behavior From fe557eee7de236e767a81a123c5de9b52d5e9a2a Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Thu, 8 Mar 2018 23:15:39 +0200 Subject: [PATCH 08/10] another rewrite based on @nikomatsakis texthg --- src/libcore/cell.rs | 46 ++++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 8accbc204c1d7..61b0aead22f20 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1161,27 +1161,43 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// The `UnsafeCell` type is the only legal way to obtain aliasable data that is considered /// mutable. In general, transmuting an `&T` type into an `&mut T` is considered undefined behavior. /// -/// The compiler makes optimizations based on the knowledge that `&T` is not mutably aliased or -/// mutated, and that `&mut T` is unique. When building abstractions like `Cell`, `RefCell`, -/// `Mutex`, etc, you need to turn these optimizations off. `UnsafeCell` is the only legal way -/// to do this. When `UnsafeCell` itself is immutably aliased, it is still safe to obtain -/// a mutable reference to its interior and/or to mutate the interior. However, the abstraction -/// designer must ensure that any active mutable references to the interior obtained this way does -/// not co-exist with other active references to the interior, either mutable or not. This is often -/// done via runtime checks. Naturally, several active immutable references to the interior can -/// co-exits with each other (but not with a mutable reference). +/// If you have a reference `&SomeStruct`, then normally in Rust all fields of `SomeStruct` are +/// immutable. The compiler makes optimizations based on the knowledge that `&T` is not mutably +/// aliased or mutated, and that `&mut T` is unique. `UnsafeCel` is the only core language +/// feature to work around this restriction. All other types that allow internal mutability, such as +/// `Cell` and `RefCell` use `UnsafeCell` to wrap their internal data. /// -/// To put it in other words, if a mutable reference to the contents is active, no other references -/// can be active at the same time, and if an immutable reference to the contents is active, then -/// only other immutable reference may be active. +/// The `UnsafeCell` API itself is technically very simple: it gives you a raw pointer `*mut T` to +/// its contents. It is up to _you_ as the abstraction designer to use that raw pointer correctly. +/// +/// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious: +/// +/// - If you create a safe reference with lifetime `'a` (either a `&T` or `&mut T` reference) that +/// is accessible by safe code (for example, because you returned it), then you must not access +/// the data in any way that contradicts that reference for the remainder of `'a`. For example, that +/// means that if you take the `*mut T` from an `UnsafeCell` and case it to an `&T`, then until +/// that reference's lifetime expires, the data in `T` must remain immutable (modulo any +/// `UnsafeCell` data found within `T`, of course). Similarly, if you create an `&mut T` reference +/// that is released to safe code, then you must not access the data within the `UnsafeCell` until +/// that reference expires. +/// +/// - At all times, you must avoid data races, meaning that if multiple threads have access to +/// the same `UnsafeCell`, then any writes must have a proper happens-before relation to all other +/// accesses (or use atomics). +/// +/// To assist with proper design, the following scenarios are explicitly declared legal +/// for single-threaded code: +/// +/// 1. A `&T` reference can be released to safe code and there it can co-exit with other `&T` +/// references, but not with a `&mut T` +/// +/// 2. A `&mut T` reference may be released to safe code, provided neither other `&mut T` nor `&T` +/// co-exist with it. A `&mut T` must always be unique. /// /// Note that while mutating or mutably aliasing the contents of an `& UnsafeCell` is /// okay (provided you enforce the invariants some other way), it is still undefined behavior /// to have multiple `&mut UnsafeCell` aliases. /// -/// -/// Types like `Cell` and `RefCell` use this type to wrap their internal data. -/// /// # Examples /// /// ``` From fbcd2f5a6a23c48416e6a948d8733f1c225acab3 Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Thu, 8 Mar 2018 23:16:31 +0200 Subject: [PATCH 09/10] tidy. Again --- src/libcore/cell.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 61b0aead22f20..98f08676722ea 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1167,7 +1167,7 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// feature to work around this restriction. All other types that allow internal mutability, such as /// `Cell` and `RefCell` use `UnsafeCell` to wrap their internal data. /// -/// The `UnsafeCell` API itself is technically very simple: it gives you a raw pointer `*mut T` to +/// The `UnsafeCell` API itself is technically very simple: it gives you a raw pointer `*mut T` to /// its contents. It is up to _you_ as the abstraction designer to use that raw pointer correctly. /// /// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious: @@ -1182,7 +1182,7 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// that reference expires. /// /// - At all times, you must avoid data races, meaning that if multiple threads have access to -/// the same `UnsafeCell`, then any writes must have a proper happens-before relation to all other +/// the same `UnsafeCell`, then any writes must have a proper happens-before relation to all other /// accesses (or use atomics). /// /// To assist with proper design, the following scenarios are explicitly declared legal From 55be28367413e01262e4fb72d4af36cee368b65d Mon Sep 17 00:00:00 2001 From: Maxim Nazarenko Date: Thu, 8 Mar 2018 23:26:27 +0200 Subject: [PATCH 10/10] and again :( --- src/libcore/cell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 98f08676722ea..4cfc34b86d09b 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1176,10 +1176,10 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// is accessible by safe code (for example, because you returned it), then you must not access /// the data in any way that contradicts that reference for the remainder of `'a`. For example, that /// means that if you take the `*mut T` from an `UnsafeCell` and case it to an `&T`, then until -/// that reference's lifetime expires, the data in `T` must remain immutable (modulo any +/// that reference's lifetime expires, the data in `T` must remain immutable (modulo any /// `UnsafeCell` data found within `T`, of course). Similarly, if you create an `&mut T` reference /// that is released to safe code, then you must not access the data within the `UnsafeCell` until -/// that reference expires. +/// that reference expires. /// /// - At all times, you must avoid data races, meaning that if multiple threads have access to /// the same `UnsafeCell`, then any writes must have a proper happens-before relation to all other @@ -1189,7 +1189,7 @@ impl<'a, T: ?Sized + fmt::Display> fmt::Display for RefMut<'a, T> { /// for single-threaded code: /// /// 1. A `&T` reference can be released to safe code and there it can co-exit with other `&T` -/// references, but not with a `&mut T` +/// references, but not with a `&mut T` /// /// 2. A `&mut T` reference may be released to safe code, provided neither other `&mut T` nor `&T` /// co-exist with it. A `&mut T` must always be unique.