Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd simple docs example for struct Cell #43423
Conversation
rust-highfive
assigned
BurntSushi
Jul 23, 2017
This comment has been minimized.
This comment has been minimized.
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @BurntSushi (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
xliiv
force-pushed the
xliiv:cell-example
branch
from
fef7864
to
236b748
Jul 23, 2017
BurntSushi
reviewed
Jul 24, 2017
| /// // WORKS, special_field is mutable because it is Cell | ||
| /// immutable.special_field.set(new_value); | ||
| /// assert_eq!(immutable.special_field.get(), new_value); | ||
| /// ``` |
This comment has been minimized.
This comment has been minimized.
BurntSushi
Jul 24, 2017
Member
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.
This comment has been minimized.
This comment has been minimized.
xliiv
Jul 24, 2017
•
Author
Contributor
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.. .
BurntSushi
added
the
S-waiting-on-author
label
Jul 24, 2017
steveklabnik
requested changes
Jul 24, 2017
|
A few nits, and the build is failing. |
| @@ -187,6 +187,34 @@ use ops::{Deref, DerefMut, CoerceUnsized}; | |||
| use ptr; | |||
|
|
|||
| /// A mutable memory location. | |||
| /// | |||
| /// # Example | |||
This comment has been minimized.
This comment has been minimized.
steveklabnik
Jul 24, 2017
Member
this should be Examples even though there's only one, for consistency
| /// | ||
| /// # Example | ||
| /// | ||
| /// Here you can see how using `Cell<T>` allows to use muttable field inside |
This comment has been minimized.
This comment has been minimized.
| /// # Example | ||
| /// | ||
| /// Here you can see how using `Cell<T>` allows to use muttable field inside | ||
| /// immutable struct (which is also called "interior mutability"). |
This comment has been minimized.
This comment has been minimized.
xliiv
added some commits
Jul 24, 2017
This comment has been minimized.
This comment has been minimized.
|
@steveklabnik done |
steveklabnik
approved these changes
Jul 24, 2017
steveklabnik
requested changes
Jul 24, 2017
|
sorry! just some last-minute white space changes; I must have missed them before? This is good to go once that's fixed; I want to make sure this is consistent with the rest of the standard library. |
| @@ -187,6 +187,29 @@ use ops::{Deref, DerefMut, CoerceUnsized}; | |||
| use ptr; | |||
|
|
|||
| /// A mutable memory location. | |||
| /// # Examples | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
xliiv
Jul 24, 2017
Author
Contributor
like here? 8286075
Because i put it at first place and it was the cause of CI failure (something related to tidy check)
This comment has been minimized.
This comment has been minimized.
steveklabnik
Jul 24, 2017
Member
yes. you had trailing whitespace; that was the only issue, not the ///s. In other words
"///"
not
"/// "
make sense?
| /// # Examples | ||
| /// Here you can see how using `Cell<T>` allows to use mutable field inside | ||
| /// immutable struct (which is also called 'interior mutability'). | ||
| /// ``` |
This comment has been minimized.
This comment has been minimized.
| /// let new_value = 100; | ||
| /// // ERROR, because my_struct is immutable | ||
| /// // immutable.regular_field = new_value; | ||
| /// // WORKS, although `my_struct` is immutable, field `special_field` is mutable because it is Cell |
This comment has been minimized.
This comment has been minimized.
| /// }; | ||
| /// | ||
| /// let new_value = 100; | ||
| /// // ERROR, because my_struct is immutable |
This comment has been minimized.
This comment has been minimized.
xliiv
force-pushed the
xliiv:cell-example
branch
from
836c474
to
04f6f9f
Jul 24, 2017
steveklabnik
reviewed
Jul 24, 2017
| /// // immutable.regular_field = new_value; | ||
| /// | ||
| /// // WORKS, although `my_struct` is immutable, field `special_field` is mutable because it is Cell | ||
| /// immutable.special_field.set(new_value); |
This comment has been minimized.
This comment has been minimized.
steveklabnik
Jul 24, 2017
Member
main isn't the issue. here's the error
01:00:45] error[E0425]: cannot find value `immutable` in this scope
[01:00:45] --> <anon>:22:1
[01:00:45] |
[01:00:45] 22 | immutable.special_field.set(new_value);
[01:00:45] | ^^^^^^^^^ not found in this scope
[01:00:45]
[01:00:45] error[E0425]: cannot find value `immutable` in this scope
[01:00:45] --> <anon>:23:12
[01:00:45] |
[01:00:45] 23 | assert_eq!(immutable.special_field.get(), new_value);
[01:00:45] | ^^^^^^^^^ not found in this scope
[01:00:45]
on this line, you use something called "immutable" but you never defined it; it's called my_struct above
This comment has been minimized.
This comment has been minimized.
xliiv
force-pushed the
xliiv:cell-example
branch
from
04f6f9f
to
553da50
Jul 24, 2017
xliiv
force-pushed the
xliiv:cell-example
branch
from
553da50
to
d429a4e
Jul 24, 2017
This comment has been minimized.
This comment has been minimized.
|
r? @steveklabnik is it ok, what do you think? |
carols10cents
added
S-waiting-on-review
and removed
S-waiting-on-author
labels
Jul 31, 2017
This comment has been minimized.
This comment has been minimized.
|
friendly ping for review @steveklabnik ! |
This comment has been minimized.
This comment has been minimized.
rust-highfive
assigned
steveklabnik
and unassigned
BurntSushi
Jul 31, 2017
steveklabnik
approved these changes
Jul 31, 2017
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@steveklabnik you are welcome :) |
xliiv commentedJul 23, 2017
No description provided.