[do not merge] core::mem::replace using core::mem::swap#154575
[do not merge] core::mem::replace using core::mem::swap#154575GrigorenkoPV wants to merge 1 commit intorust-lang:mainfrom
core::mem::replace using core::mem::swap#154575Conversation
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
16aa3b4 to
48bb310
Compare
|
Have you checked codegen for larger types? |
This comment has been minimized.
This comment has been minimized.
48bb310 to
e85ec1f
Compare
| // CHECK-NEXT: call void @llvm.memcpy | ||
| // CHECK-NEXT: ret | ||
| core::mem::take(&mut x) | ||
| } |
There was a problem hiding this comment.
Instead of removing this subtest, maybe fix the test expectation?
There was a problem hiding this comment.
From what I can tell, this is somewhat of a regression. If I understand correctly, the test checks that we just do a memcpy the pointers/sizes from x into the return value, because we no longer care about x after the function returns. But this is no longer the case after the change to mem::replace. This sounds like it might be a good place to stop right there and say "this is a regression, we're not doing it", but a maybe a perf run is still worth it?
This comment has been minimized.
This comment has been minimized.
e85ec1f to
e3bba21
Compare
|
The Miri subtree was changed cc @rust-lang/miri |
core::mem::replace using core::mem::swapcore::mem::replace using core::mem::swap
Haven't. Will. @rustbot author I also ran into a very strange failure in miri, let's see if CI gets it too. |
|
Reminder, once the PR becomes ready for a review, use |
|
The job Click to see the possible cause of the failure (guessed by this bot)For more information how to resolve CI failures of this job, visit this link. |
Brought up in and closes #154546.
Reverts #83022.
Needs a perf run.