Skip to content

Misc API doc improvements #14304

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 7 commits into from
May 20, 2014
Merged
Show file tree
Hide file tree
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
152 changes: 151 additions & 1 deletion src/libcore/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,157 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! Types that provide interior mutability.
//! Sharable mutable containers.
//!
//! Values of the `Cell` and `RefCell` types may be mutated through
//! shared references (i.e. the common `&T` type), whereas most Rust
//! types can only be mutated through unique (`&mut T`) references. We
//! say that `Cell` and `RefCell` provide *interior mutability*, in
//! contrast with typical Rust types that exhibit *inherited
//! mutability*.
Copy link
Member

Choose a reason for hiding this comment

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

👍 , haha!

//!
//! Cell types come in two flavors: `Cell` and `RefCell`. `Cell`
//! provides `get` and `set` methods that change the
//! interior value with a single method call. `Cell` though is only
//! compatible with types that implement `Copy`. For other types,
//! one must use the `RefCell` type, acquiring a write lock before
//! mutating.
//!
//! `RefCell` uses Rust's lifetimes to implement *dynamic borrowing*,
//! a process whereby one can claim temporary, exclusive, mutable
//! access to the inner value. Borrows for `RefCell`s are tracked *at
//! runtime*, unlike Rust's native reference types which are entirely
//! tracked statically, at compile time. Because `RefCell` borrows are
//! dynamic it is possible to attempt to borrow a value that is
//! already mutably borrowed; when this happens it results in task
//! failure.
//!
//! # When to choose interior mutability
//!
//! The more common inherited mutability, where one must have unique
//! access to mutate a value, is one of the key language elements that
//! enables Rust to reason strongly about pointer aliasing, statically
//! preventing crash bugs. Because of that, inherited mutability is
//! preferred, and interior mutability is something of a last
//! resort. Since cell types enable mutation where it would otherwise
//! be disallowed though, there are occassions when interior
//! mutability might be appropriate, or even *must* be used, e.g.
//!
//! * Introducing inherited mutability roots to shared types.
//! * Implementation details of logically-immutable methods.
//! * Mutating implementations of `clone`.
//!
//! ## Introducing inherited mutability roots to shared types
//!
//! Shared smart pointer types, including `Rc` and `Arc`, provide
//! containers that can be cloned and shared between multiple parties.
//! Because the contained values may be multiply-aliased, they can
//! only be borrowed as shared references, not mutable references.
//! Without cells it would be impossible to mutate data inside of
//! shared boxes at all!
//!
//! It's very common then to put a `RefCell` inside shared pointer
//! types to reintroduce mutability:
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the usage of "then" in this sentence and the previous one is a little awkward. At the very least, having it in both sounds repetitious. I would recommend removing it outright from the previous sentence ("Without cells it would be ..."). With that one removed, leaving it here sounds better.

//!
//! ```
//! extern crate collections;
//!
//! use collections::HashMap;
//! use std::cell::RefCell;
//! use std::rc::Rc;
//!
//! fn main() {
//! let shared_map: Rc<RefCell<_>> = Rc::new(RefCell::new(HashMap::new()));
//! shared_map.borrow_mut().insert("africa", 92388);
//! shared_map.borrow_mut().insert("kyoto", 11837);
//! shared_map.borrow_mut().insert("piccadilly", 11826);
//! shared_map.borrow_mut().insert("marbles", 38);
//! }
//! ```
//!
//! ## Implementation details of logically-immutable methods
//!
//! Occasionally it may be desirable not to expose in an API that
//! there is mutation happening "under the hood". This may be because
//! logically the operation is immutable, but e.g. caching forces the
//! implementation to perform mutation; or because you must employ
//! mutation to implement a trait method that was originally defined
//! to take `&self`.
//!
//! ```
//! extern crate collections;
//!
//! use collections::HashMap;
//! use std::cell::RefCell;
//!
//! struct Graph {
//! edges: HashMap<uint, uint>,
//! span_tree_cache: RefCell<Option<Vec<(uint, uint)>>>
//! }
//!
//! impl Graph {
//! fn minimum_spanning_tree(&self) -> Vec<(uint, uint)> {
//! // Create a new scope to contain the lifetime of the
//! // dynamic borrow
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to encourage using scopes to contain RefCell borrows, or is it better to encourage the use of drop() on the borrowed Ref? We need scopes for &mut, but drop() is sufficient for Ref.

Copy link
Member

Choose a reason for hiding this comment

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

I like the visual aspect of the new scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps, but there are definitely cases where drop() is more useful. The question is, do we want to suggest to readers that the scope is necessary here, like it is with &mut, or do we want to show them that drop() works?

I'm ok with either answer, I just want to make sure it's an intentional choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't care strongly but I'll rationalize this choice as: blocks are the more fundamental way for handling value lifetimes; they are built into the language, so they are more appropriate for demonstrating basic concepts.

//! {
//! // Take a reference to the inside of cache cell
//! let mut cache = self.span_tree_cache.borrow_mut();
//! if cache.is_some() {
//! return cache.get_ref().clone();
//! }
//!
//! let span_tree = self.calc_span_tree();
//! *cache = Some(span_tree);
//! }
//!
//! // Recursive call to return the just-cached value.
//! // Note that if we had not let the previous borrow
//! // of the cache fall out of scope then the subsequent
//! // recursive borrow would cause a dynamic task failure.
//! // This is the major hazard of using `RefCell`.
//! self.minimum_spanning_tree()
//! }
//! # fn calc_span_tree(&self) -> Vec<(uint, uint)> { vec![] }
//! }
//! # fn main() { }
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just annotate the code block as no_run instead of providing an empty main method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The empty main function tricks the test parser into not wrapping the whole case in a main method, which makes the extern crate statement work. I think no_run would not do that, but I'm not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm, I didn't consider that. You're probably correct. no_run most likely does not change how rustdoc wraps the code.

//! ```
//!
//! ## Mutating implementations of `clone`
//!
//! This is simply a special - but common - case of the previous:
//! hiding mutability for operations that appear to be immutable.
//! The `clone` method is expected to not change the source value, and
//! is declared to take `&self`, not `&mut self`. Therefore any
//! mutation that happens in the `clone` method must use cell
//! types. For example, `Rc` maintains its reference counts within a
//! `Cell`.
//!
//! ```
//! use std::cell::Cell;
//!
//! struct Rc<T> {
//! ptr: *mut RcBox<T>
//! }
//!
//! struct RcBox<T> {
//! value: T,
//! refcount: Cell<uint>
//! }
//!
//! impl<T> Clone for Rc<T> {
//! fn clone(&self) -> Rc<T> {
//! unsafe {
//! (*self.ptr).refcount.set((*self.ptr).refcount.get() + 1);
//! Rc { ptr: self.ptr }
//! }
//! }
//! }
//! ```
//!
// FIXME: Explain difference between Cell and RefCell
// FIXME: Downsides to interior mutability
// FIXME: Can't be shared between threads. Dynamic borrows
// FIXME: Relationship to Atomic types and RWLock

use clone::Clone;
use cmp::Eq;
Expand Down
33 changes: 22 additions & 11 deletions src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,28 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! The Rust core library
//! The Rust Core Library
//!
//! This library is meant to represent the core functionality of rust that is
//! maximally portable to other platforms. To that extent, this library has no
//! knowledge of things like allocation, threads, I/O, etc. This library is
//! built on the assumption of a few existing symbols:
//! The Rust Core Library is the dependency-free foundation of [The
//! Rust Standard Library](../std/index.html). It is the portable glue
//! between the language and its libraries, defining the intrinsic and
//! primitive building blocks of all Rust code. It links to no
//! upstream libraries, no system libraries, and no libc.
//!
//! The core library is *minimal*: it isn't even aware of heap allocation,
//! nor does it provide concurrency or I/O. These things require
//! platform integration, and this library is platform-agnostic.
//!
//! *It is not recommended to use the core library*. The stable
//! functionality of libcore is reexported from the
//! [standard library](../std/index.html). The composition of this library is
//! subject to change over time; only the interface exposed through libstd is
//! intended to be stable.
//!
//! # How to use the core library
//!
// FIXME: Fill me in with more detail when the interface settles
//! This library is built on the assumption of a few existing symbols:
//!
//! * `memcpy`, `memcmp`, `memset` - These are core memory routines which are
//! often generated by LLVM. Additionally, this library can make explicit
Expand All @@ -23,16 +39,11 @@
//! distribution.
//!
//! * `rust_begin_unwind` - This function takes three arguments, a
//! `&fmt::Arguments`, a `&str`, and a `uint. These three arguments dictate
//! `&fmt::Arguments`, a `&str`, and a `uint`. These three arguments dictate
//! the failure message, the file at which failure was invoked, and the line.
//! It is up to consumers of this core library to define this failure
//! function; it is only required to never return.
//!
//! Currently, it is *not* recommended to use the core library. The stable
//! functionality of libcore is exported directly into the
//! [standard library](../std/index.html). The composition of this library is
//! subject to change over time, only the interface exposed through libstd is
//! intended to be stable.

#![crate_id = "core#0.11.0-pre"]
#![license = "MIT/ASL2"]
Expand Down
2 changes: 1 addition & 1 deletion src/libcore/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

/*!
*
* Traits representing built-in operators, useful for overloading
* Overloadable operators
*
* Implementing these traits allows you to get an effect similar to
* overloading operators.
Expand Down
15 changes: 8 additions & 7 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
//!
//! ## Intrinsic types and operations
//!
//! The [`ptr`](../core/ptr/index.html), [`mem`](../core/mem/index.html),
//! and [`cast`](../core/cast/index.html) modules deal with unsafe pointers,
//! memory manipulation, and coercion.
//! The [`ptr`](../core/ptr/index.html) and [`mem`](../core/mem/index.html)
//! modules deal with unsafe pointers and memory manipulation.
Copy link
Contributor

Choose a reason for hiding this comment

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

You should replace the comma with "and" (and remove the trailing comma).

" The ``ptrandmem` modules deal..."

//! [`kinds`](../core/kinds/index.html) defines the special built-in traits,
//! and [`raw`](../core/raw/index.html) the runtime representation of Rust types.
//! These are some of the lowest-level building blocks of Rust
Expand Down Expand Up @@ -135,24 +134,26 @@ extern crate libc;
#[cfg(test)] pub use realstd::cmp;
#[cfg(test)] pub use realstd::ty;

#[cfg(not(test))] pub use core::cmp;
#[cfg(not(test))] pub use core::kinds;
#[cfg(not(test))] pub use core::ops;
#[cfg(not(test))] pub use core::ty;

// NB: These reexports are in the order they should be listed in rustdoc

pub use core::any;
pub use core::bool;
pub use core::cell;
pub use core::char;
pub use core::clone;
#[cfg(not(test))] pub use core::cmp;
pub use core::container;
pub use core::default;
pub use core::intrinsics;
pub use core::iter;
#[cfg(not(test))] pub use core::kinds;
pub use core::mem;
#[cfg(not(test))] pub use core::ops;
pub use core::ptr;
pub use core::raw;
pub use core::tuple;
#[cfg(not(test))] pub use core::ty;
pub use core::result;

pub use alloc::owned;
Expand Down