Skip to content

Commit

Permalink
Generate the right MIR for by use closures
Browse files Browse the repository at this point in the history
  • Loading branch information
spastorino committed Dec 26, 2024
1 parent 73fa299 commit e3c69ef
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 46 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,7 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx
);

match capture_info.capture_kind {
ty::UpvarCapture::ByValue => {
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {
self.consume_or_copy(&place_with_id, place_with_id.hir_id);
}
ty::UpvarCapture::ByRef(upvar_borrow) => {
Expand Down
38 changes: 29 additions & 9 deletions compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,9 +666,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
origin = updated.1;

let (place, capture_kind) = match capture_clause {
hir::CaptureBy::Value { .. } | hir::CaptureBy::Use { .. } => {
adjust_for_move_closure(place, capture_kind)
}
hir::CaptureBy::Value { .. } => adjust_for_move_closure(place, capture_kind),
hir::CaptureBy::Use { .. } => adjust_for_use_closure(place, capture_kind),
hir::CaptureBy::Ref => adjust_for_non_move_closure(place, capture_kind),
};

Expand Down Expand Up @@ -1303,7 +1302,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for captured_place in root_var_min_capture_list.iter() {
match captured_place.info.capture_kind {
// Only care about captures that are moved into the closure
ty::UpvarCapture::ByValue => {
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {
projections_list.push(captured_place.place.projections.as_slice());
diagnostics_info.insert(UpvarMigrationInfo::CapturingPrecise {
source_expr: captured_place.info.path_expr_id,
Expand Down Expand Up @@ -1927,7 +1926,7 @@ fn apply_capture_kind_on_capture_ty<'tcx>(
region: ty::Region<'tcx>,
) -> Ty<'tcx> {
match capture_kind {
ty::UpvarCapture::ByValue => ty,
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => ty,
ty::UpvarCapture::ByRef(kind) => Ty::new_ref(tcx, region, ty, kind.to_mutbl_lossy()),
}
}
Expand Down Expand Up @@ -2157,6 +2156,20 @@ fn adjust_for_move_closure(
(place, ty::UpvarCapture::ByValue)
}

/// Truncate deref of any reference.
fn adjust_for_use_closure(
mut place: Place<'_>,
mut kind: ty::UpvarCapture,
) -> (Place<'_>, ty::UpvarCapture) {
let first_deref = place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref);

if let Some(idx) = first_deref {
truncate_place_to_len_and_update_capture_kind(&mut place, &mut kind, idx);
}

(place, ty::UpvarCapture::ByUse)
}

/// Adjust closure capture just that if taking ownership of data, only move data
/// from enclosing stack frame.
fn adjust_for_non_move_closure(
Expand All @@ -2167,7 +2180,7 @@ fn adjust_for_non_move_closure(
place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref);

match kind {
ty::UpvarCapture::ByValue => {
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {
if let Some(idx) = contains_deref {
truncate_place_to_len_and_update_capture_kind(&mut place, &mut kind, idx);
}
Expand Down Expand Up @@ -2212,6 +2225,7 @@ fn construct_capture_kind_reason_string<'tcx>(

let capture_kind_str = match capture_info.capture_kind {
ty::UpvarCapture::ByValue => "ByValue".into(),
ty::UpvarCapture::ByUse => "ByUse".into(),
ty::UpvarCapture::ByRef(kind) => format!("{kind:?}"),
};

Expand All @@ -2233,6 +2247,7 @@ fn construct_capture_info_string<'tcx>(

let capture_kind_str = match capture_info.capture_kind {
ty::UpvarCapture::ByValue => "ByValue".into(),
ty::UpvarCapture::ByUse => "ByUse".into(),
ty::UpvarCapture::ByRef(kind) => format!("{kind:?}"),
};
format!("{place_str} -> {capture_kind_str}")
Expand Down Expand Up @@ -2328,8 +2343,11 @@ fn determine_capture_info(
// expressions.
let eq_capture_kind = match (capture_info_a.capture_kind, capture_info_b.capture_kind) {
(ty::UpvarCapture::ByValue, ty::UpvarCapture::ByValue) => true,
(ty::UpvarCapture::ByUse, ty::UpvarCapture::ByUse) => true,
(ty::UpvarCapture::ByRef(ref_a), ty::UpvarCapture::ByRef(ref_b)) => ref_a == ref_b,
(ty::UpvarCapture::ByValue, _) | (ty::UpvarCapture::ByRef(_), _) => false,
(ty::UpvarCapture::ByValue, _)
| (ty::UpvarCapture::ByUse, _)
| (ty::UpvarCapture::ByRef(_), _) => false,
};

if eq_capture_kind {
Expand All @@ -2339,8 +2357,10 @@ fn determine_capture_info(
}
} else {
// We select the CaptureKind which ranks higher based the following priority order:
// ByValue > MutBorrow > UniqueImmBorrow > ImmBorrow
// ByUse > ByValue > MutBorrow > UniqueImmBorrow > ImmBorrow
match (capture_info_a.capture_kind, capture_info_b.capture_kind) {
(ty::UpvarCapture::ByUse, _) => capture_info_a,
(_, ty::UpvarCapture::ByUse) => capture_info_b,
(ty::UpvarCapture::ByValue, _) => capture_info_a,
(_, ty::UpvarCapture::ByValue) => capture_info_b,
(ty::UpvarCapture::ByRef(ref_a), ty::UpvarCapture::ByRef(ref_b)) => {
Expand Down Expand Up @@ -2394,7 +2414,7 @@ fn truncate_place_to_len_and_update_capture_kind<'tcx>(
}

ty::UpvarCapture::ByRef(..) => {}
ty::UpvarCapture::ByValue => {}
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {}
}

place.projections.truncate(len);
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_middle/src/ty/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ pub enum UpvarCapture {
/// depending on inference.
ByValue,

/// Upvar is captured by use. This is true when the closure is labeled `use`.
ByUse,

/// Upvar is captured by reference.
ByRef(BorrowKind),
}
Expand Down Expand Up @@ -179,7 +182,7 @@ impl<'tcx> CapturedPlace<'tcx> {

pub fn is_by_ref(&self) -> bool {
match self.info.capture_kind {
ty::UpvarCapture::ByValue => false,
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => false,
ty::UpvarCapture::ByRef(..) => true,
}
}
Expand Down Expand Up @@ -215,7 +218,7 @@ impl<'tcx> TyCtxt<'tcx> {
pub fn closure_captures(self, def_id: LocalDefId) -> &'tcx [&'tcx ty::CapturedPlace<'tcx>] {
if !self.is_closure_like(def_id.to_def_id()) {
return &[];
};
}
self.closure_typeinfo(def_id).captures
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
.map(|captured_place| {
let name = captured_place.to_symbol();
match captured_place.info.capture_kind {
ty::UpvarCapture::ByValue => name,
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => name,
ty::UpvarCapture::ByRef(..) => Symbol::intern(&format!("_ref__{name}")),
}
})
Expand Down Expand Up @@ -884,7 +884,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let mut projs = closure_env_projs.clone();
projs.push(ProjectionElem::Field(FieldIdx::new(i), ty));
match capture {
ty::UpvarCapture::ByValue => {}
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {}
ty::UpvarCapture::ByRef(..) => {
projs.push(ProjectionElem::Deref);
}
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_mir_build/src/thir/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ impl<'tcx> Cx<'tcx> {
}
},

hir::ExprKind::Closure { .. } => {
hir::ExprKind::Closure(hir::Closure { .. }) => {
let closure_ty = self.typeck_results().expr_ty(expr);
let (def_id, args, movability) = match *closure_ty.kind() {
ty::Closure(def_id, args) => (def_id, UpvarArgs::Closure(args), None),
Expand Down Expand Up @@ -1269,6 +1269,17 @@ impl<'tcx> Cx<'tcx> {

match upvar_capture {
ty::UpvarCapture::ByValue => captured_place_expr,
ty::UpvarCapture::ByUse => {
let span = captured_place_expr.span;
let expr_id = self.thir.exprs.push(captured_place_expr);

Expr {
temp_lifetime: TempLifetime { temp_lifetime, backwards_incompatible },
ty: upvar_ty,
span: closure_expr.span,
kind: ExprKind::ByUse { expr: expr_id, span },
}
}
ty::UpvarCapture::ByRef(upvar_borrow) => {
let borrow_kind = match upvar_borrow {
ty::BorrowKind::Immutable => BorrowKind::Shared,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pub(crate) fn coroutine_by_move_body_def_id<'tcx>(
// this when building the field projection in the MIR body later on.
let mut parent_capture_ty = parent_capture.place.ty();
parent_capture_ty = match parent_capture.info.capture_kind {
ty::UpvarCapture::ByValue => parent_capture_ty,
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => parent_capture_ty,
ty::UpvarCapture::ByRef(kind) => Ty::new_ref(
tcx,
tcx.lifetimes.re_erased,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_passes/src/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
);
self.acc(self.exit_ln, var, ACC_READ | ACC_USE);
}
ty::UpvarCapture::ByValue => {}
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {}
}
}
}
Expand Down Expand Up @@ -1499,7 +1499,7 @@ impl<'tcx> Liveness<'_, 'tcx> {
for (&var_hir_id, min_capture_list) in closure_min_captures {
for captured_place in min_capture_list {
match captured_place.info.capture_kind {
ty::UpvarCapture::ByValue => {}
ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => {}
ty::UpvarCapture::ByRef(..) => continue,
};
let span = captured_place.get_capture_kind_span(self.ir.tcx);
Expand Down
11 changes: 10 additions & 1 deletion tests/ui/ergonomic-clones/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,20 @@

#![feature(ergonomic_clones)]

fn ergonomic_clone_closure() -> i32 {
fn ergonomic_clone_closure_no_captures() -> i32 {
let cl = use || {
1
};
cl()
}

fn ergonomic_clone_closure_with_captures() -> String {
let s = String::from("hi");

let cl = use || {
s
};
cl()
}

fn main() {}
1 change: 0 additions & 1 deletion tests/ui/feature-gates/feature-gate-ergonomic-clones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ fn ergonomic_closure_clone() {

let s3 = use || {
//~^ ERROR `.use` calls are experimental [E0658]
//~| ERROR use of moved value: `s1` [E0382]
s1
};
}
Expand Down
28 changes: 2 additions & 26 deletions tests/ui/feature-gates/feature-gate-ergonomic-clones.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,6 @@ LL | let s3 = use || {
= help: add `#![feature(ergonomic_clones)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0382]: use of moved value: `s1`
--> $DIR/feature-gate-ergonomic-clones.rs:14:14
|
LL | let s1 = String::from("hi!");
| -- move occurs because `s1` has type `String`, which does not implement the `Copy` trait
LL |
LL | let s2 = use || {
| ------ value moved into closure here
LL |
LL | s1
| -- variable moved due to use in closure
...
LL | let s3 = use || {
| ^^^^^^ value used here after move
...
LL | s1
| -- use occurs due to use in closure
|
help: consider cloning the value if the performance cost is acceptable
|
LL | s1.clone()
| ++++++++

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors

Some errors have detailed explanations: E0382, E0658.
For more information about an error, try `rustc --explain E0382`.
For more information about this error, try `rustc --explain E0658`.

0 comments on commit e3c69ef

Please sign in to comment.