diff --git a/compiler/rustc_hir_typeck/src/upvar.rs b/compiler/rustc_hir_typeck/src/upvar.rs index 4a92a3ed62e0e..d75d346e712e0 100644 --- a/compiler/rustc_hir_typeck/src/upvar.rs +++ b/compiler/rustc_hir_typeck/src/upvar.rs @@ -2344,12 +2344,22 @@ fn adjust_for_non_move_closure( place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref); match kind { - ty::UpvarCapture::ByValue | ty::UpvarCapture::ByUse => { + ty::UpvarCapture::ByValue => { if let Some(idx) = contains_deref { truncate_place_to_len_and_update_capture_kind(&mut place, &mut kind, idx); } } + // A non-`move`/`use` closure that only `.use`s an upvar does not need to + // own (and thus clone-on-capture) the value. The `ByUse` kind here can only + // come from a `x.use` in the body (a `use ||` capture clause goes through + // `adjust_for_use_closure` instead). Capturing such a place by immutable + // borrow lets the `.use` expression clone per evaluation, rather than also + // cloning the value into the closure at construction time. See #157141. + ty::UpvarCapture::ByUse => { + kind = ty::UpvarCapture::ByRef(ty::BorrowKind::Immutable); + } + ty::UpvarCapture::ByRef(..) => {} } diff --git a/tests/ui/ergonomic-clones/closure/dotuse-no-extra-capture-clone.rs b/tests/ui/ergonomic-clones/closure/dotuse-no-extra-capture-clone.rs new file mode 100644 index 0000000000000..c1662d6f4d1b8 --- /dev/null +++ b/tests/ui/ergonomic-clones/closure/dotuse-no-extra-capture-clone.rs @@ -0,0 +1,69 @@ +//! Regression test for #157141. +//! +//! A non-`move`, non-`use` closure that only `.use`s an upvar should capture it +//! by immutable borrow, not by `use` (which would clone the value into the +//! closure at construction time). The `.use` expression in the body is then the +//! only thing that clones, once per evaluation. + +//@ run-pass + +#![feature(ergonomic_clones)] +#![allow(incomplete_features)] + +use std::sync::atomic::{AtomicUsize, Ordering}; + +static CLONES: AtomicUsize = AtomicUsize::new(0); + +struct Thing; + +impl Clone for Thing { + fn clone(&self) -> Self { + CLONES.fetch_add(1, Ordering::Relaxed); + Thing + } +} + +impl std::clone::UseCloned for Thing {} + +fn clones_during(f: impl FnOnce() -> R) -> usize { + let before = CLONES.load(Ordering::Relaxed); + f(); + CLONES.load(Ordering::Relaxed) - before +} + +fn main() { + // Plain `||` closure: `x` is borrowed, so building the closure clones nothing. + // Each call clones exactly once via the `.use` in the body. + let x = Thing; + let n = clones_during(|| { + let closure = || { + let _y = x.use; + }; + closure(); + closure(); + }); + assert_eq!(n, 2, "plain `||` closure should clone once per call, not also at capture"); + drop(x); + + // `move ||` closure: `x` is moved in (no capture clone), `.use` clones per call. + let x = Thing; + let n = clones_during(|| { + let closure = move || { + let _y = x.use; + }; + closure(); + closure(); + }); + assert_eq!(n, 2, "`move ||` closure should clone once per call"); + + // `use ||` closure: the capture clause clones `x` into the closure once, at + // construction. Calling it afterwards moves the captured value out. + let x = Thing; + let n = clones_during(|| { + let closure = use || { + let _y = &x; + }; + closure(); + }); + assert_eq!(n, 1, "`use ||` closure clones once at construction"); +}