Lift: take self by value #78027
Lift: take self by value #78027
Conversation
|
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 76d5e7bb4d4c5b3978eaf3af2e625eaefa2dc135 with merge f146742c9e7558505ad9c399cba5e2ee05efa479... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued f146742c9e7558505ad9c399cba5e2ee05efa479 with parent 5a51185, future comparison URL. |
|
Finished benchmarking try commit (f146742c9e7558505ad9c399cba5e2ee05efa479): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
This is a very slight improvement in performance. (0.0%-0.1%) |
|
yeah, I guess that's about as expected. Mostly think that taking self by value here is cleaner |
camelid
added
the
C-cleanup
| @@ -686,7 +686,7 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc | |||
| // "Lift" into the tcx -- once regions are erased, this type should be in the | |||
| // global arenas; this "lift" operation basically just asserts that is true, but | |||
| // that is useful later. | |||
| tcx.lift(&drop_place_ty).unwrap(); | |||
| tcx.lift(drop_place_ty).unwrap(); | |||
There was a problem hiding this comment.
Isn't this function call fairly useless now that we only have one global context?
There was a problem hiding this comment.
Yeah, I don't know what this did but it should be unnecessary. cc @nikomatsakis
There was a problem hiding this comment.
I agree it can be removed. As the comment states, its only purpose was a kind of assertion, and I think it no longer serves even that purpose.
| let (a, b, c) = self; | ||
| tcx.lift(a).and_then(|a| tcx.lift(b).and_then(|b| tcx.lift(c).map(|c| (a, b, c)))) |
There was a problem hiding this comment.
If you're going to rewrite these, you can use ? nowadays.
E.g. Some((tcx.lift(a)?, tcx.lift(b)?, tcx.lift(c)?))
|
I have nothing against this change, FWIW. |
|
Implementation looks fine to me. r=me after addressing the two comments. |
|
@bors r=varkor |
|
📌 Commit 17825c9 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions, checks-azure |
seems small enough to not warrant an MCP 🤷