Skip to content
Merged
Changes from 4 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
23 changes: 23 additions & 0 deletions src/libcore/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,29 @@ use ops::{Deref, DerefMut, CoerceUnsized};
use ptr;

/// A mutable memory location.
/// # Examples
Copy link
Contributor

Choose a reason for hiding this comment

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

can you put a /// above and below this line

Copy link
Author

Choose a reason for hiding this comment

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

like here? 8286075

Because i put it at first place and it was the cause of CI failure (something related to tidy check)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes. you had trailing whitespace; that was the only issue, not the ///s. In other words

"///"

not

"/// "

make sense?

/// Here you can see how using `Cell<T>` allows to use mutable field inside
/// immutable struct (which is also called 'interior mutability').
/// ```
Copy link
Contributor

Choose a reason for hiding this comment

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

also above this line

/// use std::cell::Cell;
///
/// struct SomeStruct {
/// regular_field: u8,
/// special_field: Cell<u8>,
/// }
///
/// let my_struct = SomeStruct {
/// regular_field: 0,
/// special_field: Cell::new(1),
/// };
///
/// let new_value = 100;
/// // ERROR, because my_struct is immutable
Copy link
Contributor

Choose a reason for hiding this comment

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

also one above this line

/// // immutable.regular_field = new_value;
/// // WORKS, although `my_struct` is immutable, field `special_field` is mutable because it is Cell
Copy link
Contributor

Choose a reason for hiding this comment

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

also one above this line

/// immutable.special_field.set(new_value);
/// assert_eq!(immutable.special_field.get(), new_value);
/// ```
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 follows the prevailing style in the docs. For example, I think it is at least missing an Examples header. I'd also like to see some prose describing what the example is trying to show.

cc @steveklabnik

Copy link
Author

@ghost ghost Jul 24, 2017

Choose a reason for hiding this comment

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

I've added some prose and example section.
BTW: I'd see reference to this blog post:
https://ricardomartins.cc/2016/06/08/interior-mutability
in the docs. but perhaps adding links to blog post is not the way of doing docs. :)

Maybe it's just me but I was reading this https://doc.rust-lang.org/std/cell/ a few times and still didn't know what Cell is all about then read the blog post once to get the concept easily.. .

///
/// See the [module-level documentation](index.html) for more.
#[stable(feature = "rust1", since = "1.0.0")]
Expand Down