-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[Docs] Expand the section about pointer aliasing in TRPL: 4.2. Unsafe Code #21159
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
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@@ -49,32 +49,16 @@ and mutable (`&mut`) references have some aliasing and freezing | |||
guarantees, required for memory safety. | |||
|
|||
In particular, if you have an `&T` reference, then the `T` must not be | |||
modified through that reference or any other reference. There are some | |||
modified through that reference or any other reference or pointer and must not | |||
alias with any `&mut` reference. There are some | |||
standard library types, e.g. `Cell` and `RefCell`, that provide inner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it'd be nice to wrap this whole paragraph rather than have the weird line break here
I like this expasion! I'd like someone else to review this for content too, as I lack experience in this area. |
Several examples can give a better comprehension of the reference and pointer aliasing rules. | ||
|
||
First of all, using `unsafe` code to incorrectly circumvent and violate the | ||
restrictions on references leads to undefined behaviour. For example, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way to “correctly” circumvent the aliasing rules? If not, I’d probably remove the word. I’d also remove either “circumvent” or “violate”, since they’re synonymous.
First of all, using
unsafe
code to circumvent the restrictions on references leads to undefined behaviour.
Bump. r? @alexcrichton What bothers me is that the description (e.g. "Raw pointers can alias with anything") is not really true, for example if in the second example we convert the reference Should I close this PR? Or improve it somehow? My goal is to keep things practical, without precise definitions (which belong to the reference, not to the |
@alexcrichton does this mean you're happy with the content? I explicitly asked for a second reviewer upthread, I'm happy if it's accurate. |
Oh sorry, I'll take a look. |
|
||
fn main() { | ||
let mut val_cell = UnsafeCell { value: 1i32 }; | ||
unsafe { f_cell(&mut val_cell.value, &mut val_cell) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When not done in statics, the more conventional way to construct an UnsafeCell
is via UnsafeCell::new
and using the .get()
method to get out a raw pointer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this piece of code is not conventional, UnsafeCell
is explained here, not used :)
So I pursue the maximum transparency and ability to quickly substitute UnsafeCell
with a user-defined structure to see the difference in behavior.
Hm, I'm also not quite sure about this. I don't think I'm personally comfortable enough signing off on this, it's documenting some guarantees which sound reasonable, but we'd probably want to be pretty careful in this area. I suspect that we'd want to clarify documentation like this in part of a general effort to clarify the precise boundaries of unsafe code and what is/isn't possible. |
mutability by replacing compile time guarantees with dynamic checks at | ||
runtime. | ||
modified through that reference or any other reference or pointer and must not | ||
alias with any `&mut` reference. There are some standard library types, e.g. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Distinguishing between pointers and references seems a bit strange and overly verbose. It seems like we should just use a single word to refer to all pointer types. The language reference refers to them as 'pointers', but I've noticed in the past that some people in the Rust community dislike the P-word.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can adjust the text to use the term "reference" for &T/&mut T
, "raw pointer" for *const T/*mut T
and "pointer" for both references and raw pointers. Would it be ok?
To elaborate on my comment to the PR, I think it is impossible to describe correct rules here without involving lifetime variables. For example, this code from fn deref_mut<'a>(&'a mut self) -> &'a mut T {
unsafe { &mut *self._parent.value.get() }
} But this code, only differing in lifetime variables, is not: fn deref_mut<'a>(&'a mut self) -> &'static mut T {
unsafe { &mut *self._parent.value.get() }
} |
@zwarich |
Is the book a normative document? I assumed that it serves a teaching purpose and can contain acceptable degree of simplifications and half-truths (but not plain lie) compared to the formal reference. |
@petrochenkov The problem I pointed out isn't a problem of dangling references. It's about the obligations that unsafe code has to fulfill to safe code when it creates new pointers with restricted aliasing. The other major problem with these rules is that it forbids creating raw derived pointers from |
Yes, that's a problem, I continue to study the compiler, but the precise actual rules are still not clear for me - that's why I asked for possible improvements and corrections. |
Closing for now. |
The whole aliasing thing is being discussed over at rust-lang/rfcs#1447 |
It is an important topic to cover, but I'm not 100% sure I got everything right. Please, review / correct.
Somewhat related issue: #16022