-
Notifications
You must be signed in to change notification settings - Fork 14k
Allow reading a *mut without an internal cast
#111233
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| - // MIR for `ptr_offset_mut` before LowerIntrinsics | ||
| + // MIR for `ptr_offset_mut` after LowerIntrinsics | ||
|
|
||
| fn ptr_offset_mut(_1: *mut i32, _2: usize) -> *mut i32 { | ||
| debug p => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:30: +0:31 | ||
| debug d => _2; // in scope 0 at $DIR/lower_intrinsics.rs:+0:43: +0:44 | ||
| let mut _0: *mut i32; // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:56: +0:64 | ||
| let mut _3: *mut i32; // in scope 0 at $DIR/lower_intrinsics.rs:+1:30: +1:31 | ||
| let mut _4: usize; // in scope 0 at $DIR/lower_intrinsics.rs:+1:33: +1:34 | ||
|
|
||
| bb0: { | ||
| StorageLive(_3); // scope 0 at $DIR/lower_intrinsics.rs:+1:30: +1:31 | ||
| _3 = _1; // scope 0 at $DIR/lower_intrinsics.rs:+1:30: +1:31 | ||
| StorageLive(_4); // scope 0 at $DIR/lower_intrinsics.rs:+1:33: +1:34 | ||
| _4 = _2; // scope 0 at $DIR/lower_intrinsics.rs:+1:33: +1:34 | ||
| - _0 = offset::<*mut i32, usize>(move _3, move _4) -> [return: bb1, unwind unreachable]; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35 | ||
| - // mir::Constant | ||
| - // + span: $DIR/lower_intrinsics.rs:154:5: 154:29 | ||
| - // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*mut i32, usize) -> *mut i32 {offset::<*mut i32, usize>}, val: Value(<ZST>) } | ||
| + _0 = Offset(move _3, move _4); // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35 | ||
| + goto -> bb1; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:35 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly to the other comment, why do we
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separation of concerns: |
||
| } | ||
|
|
||
| bb1: { | ||
| StorageDead(_4); // scope 0 at $DIR/lower_intrinsics.rs:+1:34: +1:35 | ||
| StorageDead(_3); // scope 0 at $DIR/lower_intrinsics.rs:+1:34: +1:35 | ||
| return; // scope 0 at $DIR/lower_intrinsics.rs:+2:2: +2:2 | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| - // MIR for `read_via_copy_mut` before LowerIntrinsics | ||
| + // MIR for `read_via_copy_mut` after LowerIntrinsics | ||
|
|
||
| fn read_via_copy_mut(_1: *mut i64) -> i64 { | ||
| debug r => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:33: +0:34 | ||
| let mut _0: i64; // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:49: +0:52 | ||
| let mut _2: *mut i64; // in scope 0 at $DIR/lower_intrinsics.rs:+1:37: +1:38 | ||
|
|
||
| bb0: { | ||
| StorageLive(_2); // scope 0 at $DIR/lower_intrinsics.rs:+1:37: +1:38 | ||
| _2 = _1; // scope 0 at $DIR/lower_intrinsics.rs:+1:37: +1:38 | ||
| - _0 = read_via_copy::<*mut i64, i64>(move _2) -> [return: bb1, unwind unreachable]; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:39 | ||
| - // mir::Constant | ||
| - // + span: $DIR/lower_intrinsics.rs:129:5: 129:36 | ||
| - // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*mut i64) -> i64 {read_via_copy::<*mut i64, i64>}, val: Value(<ZST>) } | ||
| + _0 = (*_2); // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:39 | ||
| + goto -> bb1; // scope 0 at $DIR/lower_intrinsics.rs:+1:5: +1:39 | ||
| } | ||
|
|
||
| bb1: { | ||
| StorageDead(_2); // scope 0 at $DIR/lower_intrinsics.rs:+1:38: +1:39 | ||
| return; // scope 0 at $DIR/lower_intrinsics.rs:+2:2: +2:2 | ||
| } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,17 @@ | |
| fn read_via_copy_primitive(_1: &i32) -> i32 { | ||
| debug r => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:32: +0:33 | ||
| let mut _0: i32; // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:44: +0:47 | ||
| let mut _2: *const i32; // in scope 0 at $DIR/lower_intrinsics.rs:+1:46: +1:47 | ||
| let mut _2: &i32; // in scope 0 at $DIR/lower_intrinsics.rs:+1:46: +1:47 | ||
| scope 1 { | ||
| } | ||
|
|
||
| bb0: { | ||
| StorageLive(_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47 | ||
| _2 = &raw const (*_1); // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47 | ||
| - _0 = read_via_copy::<i32>(move _2) -> [return: bb1, unwind unreachable]; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48 | ||
| _2 = _1; // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47 | ||
| - _0 = read_via_copy::<&i32, i32>(move _2) -> [return: bb1, unwind unreachable]; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48 | ||
|
||
| - // mir::Constant | ||
| - // + span: $DIR/lower_intrinsics.rs:119:14: 119:45 | ||
| - // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const i32) -> i32 {read_via_copy::<i32>}, val: Value(<ZST>) } | ||
| - // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(&i32) -> i32 {read_via_copy::<&i32, i32>}, val: Value(<ZST>) } | ||
| + _0 = (*_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48 | ||
| + goto -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48 | ||
| } | ||
|
|
||
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.
Compared to
ptr::read, this just avoids a single MIR statement to turn the*mutinto a*const, right? I am quite surprised that this would be a change worth doing. And if it is worth doing, a more holistic approach might be to just not do that cast and instead allow*muteverywhere that*constis allowed?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.
This does that a bit more for the method versions --
*muthas hadreadfor 5 years now, and this PR makes that 1 statement shorter, which is certainly unimportant on its own but impacts every time it's mir-inlined. I suppose we can compatibly addimpl Traitparameters to functions now, so we could consider lettingptr::readtake more things too, but I don't know how disruptive that would be -- or if libs-api would even want it.But I'm also not certain I understand what you were imagining with "allow
*muteverywhere that*constis allowed". Could you sketch it out a bit more? I'm curious. (AFAIK MIR is typed enough that we can't omit these casts, even though they're NOPs?)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 was thinking MIR optimizations could remove the
*mutto*constcast that was introduced by MIR building, and our notion of "well-typed MIR" could be weakened to say that this is fine.