diff options
9 files changed, 110 insertions, 57 deletions
diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 9a39a32f6d5..39874f48eb0 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -1528,20 +1528,11 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { &mut self, place_with_id: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId, - mode: euv::ConsumeMode, ) { debug!( - "adjust_upvar_borrow_kind_for_consume(place_with_id={:?}, diag_expr_id={:?}, mode={:?})", - place_with_id, diag_expr_id, mode + "adjust_upvar_borrow_kind_for_consume(place_with_id={:?}, diag_expr_id={:?})", + place_with_id, diag_expr_id ); - - // Copy type being used as ByValue are equivalent to ImmBorrow and don't require any - // escalation. - match mode { - euv::ConsumeMode::Copy => return, - euv::ConsumeMode::Move => {} - }; - let tcx = self.fcx.tcx; let upvar_id = if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base { upvar_id @@ -1716,22 +1707,14 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { } } - fn consume( - &mut self, - place_with_id: &PlaceWithHirId<'tcx>, - diag_expr_id: hir::HirId, - mode: euv::ConsumeMode, - ) { - debug!( - "consume(place_with_id={:?}, diag_expr_id={:?}, mode={:?})", - place_with_id, diag_expr_id, mode - ); + fn consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) { + debug!("consume(place_with_id={:?}, diag_expr_id={:?})", place_with_id, diag_expr_id); if !self.capture_information.contains_key(&place_with_id.place) { self.init_capture_info_for_place(&place_with_id, diag_expr_id); } - self.adjust_upvar_borrow_kind_for_consume(&place_with_id, diag_expr_id, mode); + self.adjust_upvar_borrow_kind_for_consume(&place_with_id, diag_expr_id); } fn borrow( diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index f1749412794..a2bb420a901 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -2,8 +2,6 @@ //! normal visitor, which just walks the entire body in one shot, the //! `ExprUseVisitor` determines how expressions are being used. -pub use self::ConsumeMode::*; - // Export these here so that Clippy can use them. pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection}; @@ -28,19 +26,20 @@ use crate::mem_categorization as mc; /// This trait defines the callbacks you can expect to receive when /// employing the ExprUseVisitor. pub trait Delegate<'tcx> { - // The value found at `place` is either copied or moved, depending + // The value found at `place` is moved, depending // on `mode`. Where `diag_expr_id` is the id used for diagnostics for `place`. // + // Use of a `Copy` type in a ByValue context is considered a use + // by `ImmBorrow` and `borrow` is called instead. This is because + // a shared borrow is the "minimum access" that would be needed + // to perform a copy. + // + // // The parameter `diag_expr_id` indicates the HIR id that ought to be used for // diagnostics. Around pattern matching such as `let pat = expr`, the diagnostic // id will be the id of the expression `expr` but the place itself will have // the id of the binding in the pattern `pat`. - fn consume( - &mut self, - place_with_id: &PlaceWithHirId<'tcx>, - diag_expr_id: hir::HirId, - mode: ConsumeMode, - ); + fn consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId); // The value found at `place` is being borrowed with kind `bk`. // `diag_expr_id` is the id used for diagnostics (see `consume` for more details). @@ -60,7 +59,7 @@ pub trait Delegate<'tcx> { } #[derive(Copy, Clone, PartialEq, Debug)] -pub enum ConsumeMode { +enum ConsumeMode { Copy, // reference to x where x has a type that copies Move, // reference to x where x has a type that moves } @@ -141,10 +140,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } fn delegate_consume(&mut self, place_with_id: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) { - debug!("delegate_consume(place_with_id={:?})", place_with_id); - - let mode = copy_or_move(&self.mc, place_with_id); - self.delegate.consume(place_with_id, diag_expr_id, mode); + delegate_consume(&self.mc, self.delegate, place_with_id, diag_expr_id) } fn consume_exprs(&mut self, exprs: &[hir::Expr<'_>]) { @@ -653,9 +649,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { delegate.borrow(place, discr_place.hir_id, bk); } ty::BindByValue(..) => { - let mode = copy_or_move(mc, &place); debug!("walk_pat binding consuming pat"); - delegate.consume(place, discr_place.hir_id, mode); + delegate_consume(mc, *delegate, place, discr_place.hir_id); } } } @@ -773,8 +768,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { match capture_info.capture_kind { ty::UpvarCapture::ByValue(_) => { - let mode = copy_or_move(&self.mc, &place_with_id); - self.delegate.consume(&place_with_id, place_with_id.hir_id, mode); + self.delegate_consume(&place_with_id, place_with_id.hir_id); } ty::UpvarCapture::ByRef(upvar_borrow) => { self.delegate.borrow( @@ -798,8 +792,28 @@ fn copy_or_move<'a, 'tcx>( place_with_id.place.ty(), mc.tcx().hir().span(place_with_id.hir_id), ) { - Move + ConsumeMode::Move } else { - Copy + ConsumeMode::Copy + } +} + +// - If a place is used in a `ByValue` context then move it if it's not a `Copy` type. +// - If the place that is a `Copy` type consider it a `ImmBorrow`. +fn delegate_consume<'a, 'tcx>( + mc: &mc::MemCategorizationContext<'a, 'tcx>, + delegate: &mut (dyn Delegate<'tcx> + 'a), + place_with_id: &PlaceWithHirId<'tcx>, + diag_expr_id: hir::HirId, +) { + debug!("delegate_consume(place_with_id={:?})", place_with_id); + + let mode = copy_or_move(&mc, place_with_id); + + match mode { + ConsumeMode::Move => delegate.consume(place_with_id, diag_expr_id), + ConsumeMode::Copy => { + delegate.borrow(place_with_id, diag_expr_id, ty::BorrowKind::ImmBorrow) + } } } diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/move_closure.rs index 3b284eadbd0..0e7abf64fab 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.rs @@ -195,6 +195,21 @@ fn box_mut_2() { //~| NOTE: Min Capture p_foo[Deref,Deref,(0, 0)] -> UniqueImmBorrow } +// Test that move closures can take ownership of Copy type +fn returned_closure_owns_copy_type_data() -> impl Fn() -> i32 { + let x = 10; + + let c = #[rustc_capture_analysis] move || x; + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> + //~| First Pass analysis includes: + //~| NOTE: Capturing x[] -> ImmBorrow + //~| Min Capture analysis includes: + //~| NOTE: Min Capture x[] -> ByValue + + c +} + fn main() { simple_move_closure(); simple_ref(); diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr index c8e2708feee..82ed99f9444 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr @@ -88,6 +88,39 @@ LL | let c = #[rustc_capture_analysis] move || p_foo.x += 10; = note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:202:13 + | +LL | let c = #[rustc_capture_analysis] move || x; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:202:39 + | +LL | let c = #[rustc_capture_analysis] move || x; + | ^^^^^^^^^ + | +note: Capturing x[] -> ImmBorrow + --> $DIR/move_closure.rs:202:47 + | +LL | let c = #[rustc_capture_analysis] move || x; + | ^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:202:39 + | +LL | let c = #[rustc_capture_analysis] move || x; + | ^^^^^^^^^ + | +note: Min Capture x[] -> ByValue + --> $DIR/move_closure.rs:202:47 + | +LL | let c = #[rustc_capture_analysis] move || x; + | ^ + error: First Pass analysis includes: --> $DIR/move_closure.rs:15:5 | @@ -424,6 +457,6 @@ note: Min Capture p_foo[Deref,Deref,(0, 0)] -> UniqueImmBorrow LL | let c = #[rustc_capture_analysis] move || p_foo.x += 10; | ^^^^^^^ -error: aborting due to 30 previous errors +error: aborting due to 33 previous errors For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs index 65c8a5a7850..e1b61e85ec1 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs @@ -3,6 +3,8 @@ // Test that move closures compile properly with `capture_disjoint_fields` enabled. +#![allow(unused)] + fn simple_ref() { let mut s = 10; let ref_s = &mut s; @@ -92,6 +94,15 @@ fn data_moved_but_not_fn_once() { c(); } +// Test that move closures can take ownership of Copy type +fn returned_closure_owns_copy_type_data() -> impl Fn() -> i32 { + let x = 10; + + let c = move || x; + + c +} + fn main() { simple_ref(); struct_contains_ref_to_another_struct(); @@ -100,4 +111,6 @@ fn main() { disjoint_via_ref(); data_moved_but_not_fn_once(); + + returned_closure_owns_copy_type_data(); } diff --git a/src/tools/clippy/clippy_lints/src/escape.rs b/src/tools/clippy/clippy_lints/src/escape.rs index 3581ab41906..5f400d079da 100644 --- a/src/tools/clippy/clippy_lints/src/escape.rs +++ b/src/tools/clippy/clippy_lints/src/escape.rs @@ -11,7 +11,7 @@ use rustc_span::source_map::Span; use rustc_span::symbol::kw; use rustc_target::abi::LayoutOf; use rustc_target::spec::abi::Abi; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; +use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; #[derive(Copy, Clone)] pub struct BoxedLocal { @@ -133,13 +133,10 @@ fn is_argument(map: rustc_middle::hir::map::Map<'_>, id: HirId) -> bool { } impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { - fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, mode: ConsumeMode) { + fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) { if cmt.place.projections.is_empty() { if let PlaceBase::Local(lid) = cmt.place.base { - if let ConsumeMode::Move = mode { - // moved out or in. clearly can't be localized - self.set.remove(&lid); - } + self.set.remove(&lid); let map = &self.cx.tcx.hir(); if let Some(Node::Binding(_)) = map.find(cmt.hir_id) { if self.set.contains(&lid) { diff --git a/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs b/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs index d07b5a93b67..1e54a1e2de1 100644 --- a/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs +++ b/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs @@ -7,7 +7,7 @@ use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::{mir::FakeReadCause, ty}; use rustc_span::source_map::Span; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; +use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) { if let Some(higher::Range { @@ -82,7 +82,7 @@ struct MutatePairDelegate<'a, 'tcx> { } impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> { - fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {} + fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index e33a33e2386..57fd03f4e12 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -326,10 +326,8 @@ impl MovedVariablesCtxt { } impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { - fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId, mode: euv::ConsumeMode) { - if let euv::ConsumeMode::Move = mode { - self.move_common(cmt); - } + fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _: HirId) { + self.move_common(cmt); } fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {} diff --git a/src/tools/clippy/clippy_utils/src/usage.rs b/src/tools/clippy/clippy_utils/src/usage.rs index 182d8cb11ea..ac885e99944 100644 --- a/src/tools/clippy/clippy_utils/src/usage.rs +++ b/src/tools/clippy/clippy_utils/src/usage.rs @@ -10,7 +10,7 @@ use rustc_lint::LateContext; use rustc_middle::hir::map::Map; use rustc_middle::mir::FakeReadCause; use rustc_middle::ty; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; +use rustc_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. pub fn mutated_variables<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> Option<HirIdSet> { @@ -67,7 +67,7 @@ impl<'tcx> MutVarsDelegate { } impl<'tcx> Delegate<'tcx> for MutVarsDelegate { - fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {} + fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {} fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, bk: ty::BorrowKind) { if let ty::BorrowKind::MutBorrow = bk { |