mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #87069 - sexxi-goose:copy_ref_always, r=nikomatsakis

Browse Source 
ExprUseVisitor: Treat ByValue use of Copy types as ImmBorrow

r? ```@nikomatsakis```
This commit is contained in:
Guillaume Gomez 2021-07-16 10:08:05 +02:00 committed by GitHub
commit c1ffca0869
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 110 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
compiler/rustc_typeck/src/check/upvar.rs
View File

@ -1528,20 +1528,11 @@ fn adjust_upvar_borrow_kind_for_consume(
&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 @@ fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id:
}
}
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(

54
compiler/rustc_typeck/src/expr_use_visitor.rs
View File

@ -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 @@
/// 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 @@ fn borrow(
}
#[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 @@ fn tcx(&self) -> TyCtxt<'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 @@ fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
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 @@ fn upvar_is_local_variable(
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)
}
}
}

15
src/test/ui/closures/2229_closure_analysis/move_closure.rs
View File

@ -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();

35
src/test/ui/closures/2229_closure_analysis/move_closure.stderr
View File

@ -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`.

13
src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs
View File

@ -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();
}

9
src/tools/clippy/clippy_lints/src/escape.rs
View File

@ -11,7 +11,7 @@
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) {

4
src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs
View File

@ -7,7 +7,7 @@
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 {

6
src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs
View File

@ -326,10 +326,8 @@ fn move_common(&mut self, cmt: &euv::PlaceWithHirId<'_>) {
}
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) {}

4
src/tools/clippy/clippy_utils/src/usage.rs
View File

@ -10,7 +10,7 @@
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 @@ fn update(&mut self, cat: &PlaceWithHirId<'tcx>) {
}
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 {