From bf4bdd95c3843ef1922e62ef5ace146dae8f43a7 Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Mon, 28 Dec 2020 06:42:21 -0500
Subject: [PATCH 1/7] Add lint for 2229 migrations
---
compiler/rustc_lint_defs/src/builtin.rs | 46 +++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
index a8bf1ce51bb74..f1e5a4d47c787 100644
--- a/compiler/rustc_lint_defs/src/builtin.rs
+++ b/compiler/rustc_lint_defs/src/builtin.rs
@@ -2968,6 +2968,7 @@ declare_lint_pass! {
UNSUPPORTED_NAKED_FUNCTIONS,
MISSING_ABI,
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
+ DISJOINT_CAPTURE_DROP_REORDER,
]
}
@@ -2994,6 +2995,51 @@ declare_lint! {
"detects doc comments that aren't used by rustdoc"
}
+declare_lint! {
+ /// The `disjoint_capture_drop_reorder` lint detects variables that aren't completely
+ /// captured when the feature `capture_disjoint_fields` is enabled and it affects the Drop
+ /// order of at least one path starting at this variable.
+ ///
+ /// ### Example
+ ///
+ /// ```rust
+ /// # #![deny(disjoint_capture_drop_reorder)]
+ /// # #![allow(unused)]
+ /// struct FancyInteger(i32);
+ ///
+ /// impl Drop for FancyInteger {
+ /// fn drop(&mut self) {
+ /// println!("Just dropped {}", self.0);
+ /// }
+ /// }
+ ///
+ /// struct Point { x: FancyInteger, y: FancyInteger }
+ ///
+ /// fn main() {
+ /// let p = Point { x: FancyInteger(10), y: FancyInteger(20) };
+ ///
+ /// let c = || {
+ /// let x = p.x;
+ /// };
+ ///
+ /// c();
+ ///
+ /// // ... More code ...
+ /// }
+ /// ```
+ ///
+ /// {{produces}}
+ ///
+ /// ### Explanation
+ ///
+ /// In the above example `p.y` will be dropped at the end of `f` instead of with `c` if
+ /// the feature `capture_disjoint_fields` is enabled.
+ pub DISJOINT_CAPTURE_DROP_REORDER,
+ Allow,
+ "Drop reorder because of `capture_disjoint_fields`"
+
+}
+
declare_lint_pass!(UnusedDocComment => [UNUSED_DOC_COMMENTS]);
declare_lint! {
From d3e85014a712e2b41ac44d71ddee3d55711a8d44 Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Sat, 21 Nov 2020 04:23:42 -0500
Subject: [PATCH 2/7] Process mentioned upvars for analysis first pass after
ExprUseVisitor
- This allows us add fake information after handling migrations if
needed.
- Capture analysis also priortizes what we see earlier, which means
fake information should go in last.
---
compiler/rustc_typeck/src/check/upvar.rs | 107 ++++++++++--------
.../escape-upvar-nested.stderr | 8 +-
.../escape-upvar-ref.stderr | 4 +-
3 files changed, 68 insertions(+), 51 deletions(-)
diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs
index f039445bf7780..56b91ee4bcbd3 100644
--- a/compiler/rustc_typeck/src/check/upvar.rs
+++ b/compiler/rustc_typeck/src/check/upvar.rs
@@ -40,7 +40,7 @@ use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_infer::infer::UpvarRegion;
use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, ProjectionKind};
-use rustc_middle::ty::{self, Ty, TyCtxt, UpvarSubsts};
+use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults, UpvarSubsts};
use rustc_span::sym;
use rustc_span::{MultiSpan, Span, Symbol};
@@ -55,6 +55,11 @@ enum PlaceAncestryRelation {
Divergent,
}
+/// Intermediate format to store a captured `Place` and associated `ty::CaptureInfo`
+/// during capture analysis. Information in this map feeds into the minimum capture
+/// analysis pass.
+type InferredCaptureInformation<'tcx> = FxIndexMap<Place<'tcx>, ty::CaptureInfo<'tcx>>;
+
impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn closure_analyze(&self, body: &'tcx hir::Body<'tcx>) {
InferBorrowKindVisitor { fcx: self }.visit_body(body);
@@ -124,28 +129,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let local_def_id = closure_def_id.expect_local();
- let mut capture_information: FxIndexMap<Place<'tcx>, ty::CaptureInfo<'tcx>> =
- Default::default();
- if !self.tcx.features().capture_disjoint_fields {
- if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
- for (&var_hir_id, _) in upvars.iter() {
- let place = self.place_for_root_variable(local_def_id, var_hir_id);
-
- debug!("seed place {:?}", place);
-
- let upvar_id = ty::UpvarId::new(var_hir_id, local_def_id);
- let capture_kind = self.init_capture_kind(capture_clause, upvar_id, span);
- let info = ty::CaptureInfo {
- capture_kind_expr_id: None,
- path_expr_id: None,
- capture_kind,
- };
-
- capture_information.insert(place, info);
- }
- }
- }
-
let body_owner_def_id = self.tcx.hir().body_owner_def_id(body.id());
assert_eq!(body_owner_def_id.to_def_id(), closure_def_id);
let mut delegate = InferBorrowKind {
@@ -155,7 +138,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
capture_clause,
current_closure_kind: ty::ClosureKind::LATTICE_BOTTOM,
current_origin: None,
- capture_information,
+ capture_information: Default::default(),
};
euv::ExprUseVisitor::new(
&mut delegate,
@@ -172,6 +155,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
self.log_capture_analysis_first_pass(closure_def_id, &delegate.capture_information, span);
+ self.compute_min_captures(closure_def_id, delegate.capture_information);
+
+ // We now fake capture information for all variables that are mentioned within the closure
+ // We do this after handling migrations so that min_captures computes before
+ if !self.tcx.features().capture_disjoint_fields {
+ let mut capture_information: InferredCaptureInformation<'tcx> = Default::default();
+
+ if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
+ for var_hir_id in upvars.keys() {
+ let place = self.place_for_root_variable(local_def_id, *var_hir_id);
+
+ debug!("seed place {:?}", place);
+
+ let upvar_id = ty::UpvarId::new(*var_hir_id, local_def_id);
+ let capture_kind = self.init_capture_kind(capture_clause, upvar_id, span);
+ let fake_info = ty::CaptureInfo {
+ capture_kind_expr_id: None,
+ path_expr_id: None,
+ capture_kind,
+ };
+
+ capture_information.insert(place, fake_info);
+ }
+ }
+
+ // This will update the min captures based on this new fake information.
+ self.compute_min_captures(closure_def_id, capture_information);
+ }
+
if let Some(closure_substs) = infer_kind {
// Unify the (as yet unbound) type variable in the closure
// substs with the kind we inferred.
@@ -197,7 +209,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
- self.compute_min_captures(closure_def_id, delegate);
self.log_closure_min_capture_info(closure_def_id, span);
self.min_captures_to_closure_captures_bridge(closure_def_id);
@@ -344,6 +355,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Places (and corresponding capture kind) that we need to keep track of to support all
/// the required captured paths.
///
+ ///
+ /// Note: If this function is called multiple times for the same closure, it will update
+ /// the existing min_capture map that is stored in TypeckResults.
+ ///
/// Eg:
/// ```rust,no_run
/// struct Point { x: i32, y: i32 }
@@ -408,11 +423,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn compute_min_captures(
&self,
closure_def_id: DefId,
- inferred_info: InferBorrowKind<'_, 'tcx>,
+ capture_information: InferredCaptureInformation<'tcx>,
) {
- let mut root_var_min_capture_list: ty::RootVariableMinCaptureList<'_> = Default::default();
+ if capture_information.is_empty() {
+ return;
+ }
+
+ let mut typeck_results = self.typeck_results.borrow_mut();
- for (place, capture_info) in inferred_info.capture_information.into_iter() {
+ let mut root_var_min_capture_list =
+ typeck_results.closure_min_captures.remove(&closure_def_id).unwrap_or_default();
+
+ for (place, capture_info) in capture_information.into_iter() {
let var_hir_id = match place.base {
PlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
base => bug!("Expected upvar, found={:?}", base),
@@ -422,7 +444,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let min_cap_list = match root_var_min_capture_list.get_mut(&var_hir_id) {
None => {
- let mutability = self.determine_capture_mutability(&place);
+ let mutability = self.determine_capture_mutability(&typeck_results, &place);
let min_cap_list =
vec![ty::CapturedPlace { place, info: capture_info, mutability }];
root_var_min_capture_list.insert(var_hir_id, min_cap_list);
@@ -487,7 +509,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Only need to insert when we don't have an ancestor in the existing min capture list
if !ancestor_found {
- let mutability = self.determine_capture_mutability(&place);
+ let mutability = self.determine_capture_mutability(&typeck_results, &place);
let captured_place =
ty::CapturedPlace { place, info: updated_capture_info, mutability };
min_cap_list.push(captured_place);
@@ -495,13 +517,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
debug!("For closure={:?}, min_captures={:#?}", closure_def_id, root_var_min_capture_list);
-
- if !root_var_min_capture_list.is_empty() {
- self.typeck_results
- .borrow_mut()
- .closure_min_captures
- .insert(closure_def_id, root_var_min_capture_list);
- }
+ typeck_results.closure_min_captures.insert(closure_def_id, root_var_min_capture_list);
}
fn init_capture_kind(
@@ -613,18 +629,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// A captured place is mutable if
/// 1. Projections don't include a Deref of an immut-borrow, **and**
/// 2. PlaceBase is mut or projections include a Deref of a mut-borrow.
- fn determine_capture_mutability(&self, place: &Place<'tcx>) -> hir::Mutability {
+ fn determine_capture_mutability(
+ &self,
+ typeck_results: &'a TypeckResults<'tcx>,
+ place: &Place<'tcx>,
+ ) -> hir::Mutability {
let var_hir_id = match place.base {
PlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
_ => unreachable!(),
};
- let bm = *self
- .typeck_results
- .borrow()
- .pat_binding_modes()
- .get(var_hir_id)
- .expect("missing binding mode");
+ let bm = *typeck_results.pat_binding_modes().get(var_hir_id).expect("missing binding mode");
let mut is_mutbl = match bm {
ty::BindByValue(mutability) => mutability,
@@ -698,9 +713,11 @@ struct InferBorrowKind<'a, 'tcx> {
///
/// For closure `fix_s`, (at a high level) the map contains
///
+ /// ```
/// Place { V1, [ProjectionKind::Field(Index=0, Variant=0)] } : CaptureKind { E1, ImmutableBorrow }
/// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow }
- capture_information: FxIndexMap<Place<'tcx>, ty::CaptureInfo<'tcx>>,
+ /// ```
+ capture_information: InferredCaptureInformation<'tcx>,
}
impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
diff --git a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr
index 1a8258376142a..e1b446fc61f61 100644
--- a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr
+++ b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr
@@ -7,10 +7,10 @@ LL | let mut closure1 = || p = &y;
= note: defining type: test::{closure#0}::{closure#0} with closure substs [
i16,
extern "rust-call" fn(()),
- (&'_#1r i32, &'_#2r mut &'_#3r i32),
+ (&'_#1r mut &'_#2r i32, &'_#3r i32),
]
= note: number of external vids: 4
- = note: where '_#1r: '_#3r
+ = note: where '_#3r: '_#2r
note: external requirements
--> $DIR/escape-upvar-nested.rs:20:27
@@ -25,10 +25,10 @@ LL | | };
= note: defining type: test::{closure#0} with closure substs [
i16,
extern "rust-call" fn(()),
- (&'_#1r i32, &'_#2r mut &'_#3r i32),
+ (&'_#1r mut &'_#2r i32, &'_#3r i32),
]
= note: number of external vids: 4
- = note: where '_#1r: '_#3r
+ = note: where '_#3r: '_#2r
note: no external requirements
--> $DIR/escape-upvar-nested.rs:13:1
diff --git a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr
index 29fd796882b6a..0ea1076c32ef4 100644
--- a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr
+++ b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr
@@ -7,10 +7,10 @@ LL | let mut closure = || p = &y;
= note: defining type: test::{closure#0} with closure substs [
i16,
extern "rust-call" fn(()),
- (&'_#1r i32, &'_#2r mut &'_#3r i32),
+ (&'_#1r mut &'_#2r i32, &'_#3r i32),
]
= note: number of external vids: 4
- = note: where '_#1r: '_#3r
+ = note: where '_#3r: '_#2r
note: no external requirements
--> $DIR/escape-upvar-ref.rs:17:1
From cc5e6db5f2d25fb1c0371455574db7aecbe0ba0c Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Tue, 15 Dec 2020 03:38:15 -0500
Subject: [PATCH 3/7] Migrations first pass
---
compiler/rustc_typeck/src/check/upvar.rs | 132 ++++++++++++++++++-
compiler/rustc_typeck/src/check/writeback.rs | 6 +-
2 files changed, 134 insertions(+), 4 deletions(-)
diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs
index 56b91ee4bcbd3..16aaa48ce17fe 100644
--- a/compiler/rustc_typeck/src/check/upvar.rs
+++ b/compiler/rustc_typeck/src/check/upvar.rs
@@ -30,6 +30,7 @@
//! then mean that all later passes would have to check for these figments
//! and report an error, and it just seems like more mess in the end.)
+use super::writeback::Resolver;
use super::FnCtxt;
use crate::expr_use_visitor as euv;
@@ -40,7 +41,9 @@ use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_infer::infer::UpvarRegion;
use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, ProjectionKind};
+use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults, UpvarSubsts};
+use rustc_session::lint;
use rustc_span::sym;
use rustc_span::{MultiSpan, Span, Symbol};
@@ -97,7 +100,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
closure_hir_id: hir::HirId,
span: Span,
- body: &hir::Body<'_>,
+ body: &'tcx hir::Body<'tcx>,
capture_clause: hir::CaptureBy,
) {
debug!("analyze_closure(id={:?}, body.id={:?})", closure_hir_id, body.id());
@@ -157,6 +160,38 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.compute_min_captures(closure_def_id, delegate.capture_information);
+ let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id);
+ if should_do_migration_analysis(self.tcx, closure_hir_id) {
+ let need_migrations = self.compute_2229_migrations_first_pass(
+ closure_def_id,
+ span,
+ capture_clause,
+ body,
+ self.typeck_results.borrow().closure_min_captures.get(&closure_def_id),
+ );
+
+ if !need_migrations.is_empty() {
+ let need_migrations_hir_id =
+ need_migrations.iter().map(|m| m.0).collect::<Vec<_>>();
+
+ let migrations_text =
+ migration_suggestion_for_2229(self.tcx, &need_migrations_hir_id);
+
+ self.tcx.struct_span_lint_hir(
+ lint::builtin::DISJOINT_CAPTURE_DROP_REORDER,
+ closure_hir_id,
+ span,
+ |lint| {
+ let mut diagnostics_builder = lint.build(
+ "drop order affected for closure because of `capture_disjoint_fields`",
+ );
+ diagnostics_builder.note(&migrations_text);
+ diagnostics_builder.emit();
+ },
+ );
+ }
+ }
+
// We now fake capture information for all variables that are mentioned within the closure
// We do this after handling migrations so that min_captures computes before
if !self.tcx.features().capture_disjoint_fields {
@@ -520,6 +555,86 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
typeck_results.closure_min_captures.insert(closure_def_id, root_var_min_capture_list);
}
+ /// Figures out the list of root variables (and their types) that aren't completely
+ /// captured by the closure when `capture_disjoint_fields` is enabled and drop order of
+ /// some path starting at that root variable **might** be affected.
+ ///
+ /// The output list would include a root variable if:
+ /// - It would have been moved into the closure when `capture_disjoint_fields` wasn't
+ /// enabled, **and**
+ /// - It wasn't completely captured by the closure, **and**
+ /// - The type of the root variable needs Drop.
+ fn compute_2229_migrations_first_pass(
+ &self,
+ closure_def_id: DefId,
+ closure_span: Span,
+ closure_clause: hir::CaptureBy,
+ body: &'tcx hir::Body<'tcx>,
+ min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>,
+ ) -> Vec<(hir::HirId, Ty<'tcx>)> {
+ fn resolve_ty<T: TypeFoldable<'tcx>>(
+ fcx: &FnCtxt<'_, 'tcx>,
+ span: Span,
+ body: &'tcx hir::Body<'tcx>,
+ ty: T,
+ ) -> T {
+ let mut resolver = Resolver::new(fcx, &span, body);
+ ty.fold_with(&mut resolver)
+ }
+
+ let upvars = if let Some(upvars) = self.tcx.upvars_mentioned(closure_def_id) {
+ upvars
+ } else {
+ return vec![];
+ };
+
+ let mut need_migrations = Vec::new();
+
+ for (&var_hir_id, _) in upvars.iter() {
+ let ty = resolve_ty(self, closure_span, body, self.node_ty(var_hir_id));
+
+ if !ty.needs_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) {
+ continue;
+ }
+
+ let root_var_min_capture_list = if let Some(root_var_min_capture_list) =
+ min_captures.and_then(|m| m.get(&var_hir_id))
+ {
+ root_var_min_capture_list
+ } else {
+ // The upvar is mentioned within the closure but no path starting from it is
+ // used.
+
+ match closure_clause {
+ // Only migrate if closure is a move closure
+ hir::CaptureBy::Value => need_migrations.push((var_hir_id, ty)),
+
+ hir::CaptureBy::Ref => {}
+ }
+
+ continue;
+ };
+
+ let is_moved = root_var_min_capture_list
+ .iter()
+ .find(|capture| matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue(_)))
+ .is_some();
+
+ // 1. If we capture more than one path starting at the root variabe then the root variable
+ // isn't being captured in its entirety
+ // 2. If we only capture one path starting at the root variable, it's still possible
+ // that it isn't the root variable completely.
+ if is_moved
+ && ((root_var_min_capture_list.len() > 1)
+ || (root_var_min_capture_list[0].place.projections.len() > 0))
+ {
+ need_migrations.push((var_hir_id, ty));
+ }
+ }
+
+ need_migrations
+ }
+
fn init_capture_kind(
&self,
capture_clause: hir::CaptureBy,
@@ -1136,6 +1251,21 @@ fn var_name(tcx: TyCtxt<'_>, var_hir_id: hir::HirId) -> Symbol {
tcx.hir().name(var_hir_id)
}
+fn should_do_migration_analysis(tcx: TyCtxt<'_>, closure_id: hir::HirId) -> bool {
+ let (level, _) =
+ tcx.lint_level_at_node(lint::builtin::DISJOINT_CAPTURE_DROP_REORDER, closure_id);
+
+ !matches!(level, lint::Level::Allow)
+}
+
+fn migration_suggestion_for_2229(tcx: TyCtxt<'_>, need_migrations: &Vec<hir::HirId>) -> String {
+ let need_migrations_strings =
+ need_migrations.iter().map(|v| format!("{}", var_name(tcx, *v))).collect::<Vec<_>>();
+ let migrations_list_concat = need_migrations_strings.join(", ");
+
+ format!("let ({}) = ({});", migrations_list_concat, migrations_list_concat)
+}
+
/// Helper function to determine if we need to escalate CaptureKind from
/// CaptureInfo A to B and returns the escalated CaptureInfo.
/// (Note: CaptureInfo contains CaptureKind and an expression that led to capture it in that way)
diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs
index b6d740a4fdb57..4d18b2cb3fc49 100644
--- a/compiler/rustc_typeck/src/check/writeback.rs
+++ b/compiler/rustc_typeck/src/check/writeback.rs
@@ -650,7 +650,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
}
}
-trait Locatable {
+crate trait Locatable {
fn to_span(&self, tcx: TyCtxt<'_>) -> Span;
}
@@ -668,7 +668,7 @@ impl Locatable for hir::HirId {
/// The Resolver. This is the type folding engine that detects
/// unresolved types and so forth.
-struct Resolver<'cx, 'tcx> {
+crate struct Resolver<'cx, 'tcx> {
tcx: TyCtxt<'tcx>,
infcx: &'cx InferCtxt<'cx, 'tcx>,
span: &'cx dyn Locatable,
@@ -679,7 +679,7 @@ struct Resolver<'cx, 'tcx> {
}
impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
- fn new(
+ crate fn new(
fcx: &'cx FnCtxt<'cx, 'tcx>,
span: &'cx dyn Locatable,
body: &'tcx hir::Body<'tcx>,
From 3c71a7b60f8c661899a4e0cbe498ac67f3f02632 Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Tue, 29 Dec 2020 23:43:44 -0500
Subject: [PATCH 4/7] Tests for 2229 lint
---
.../migrations/insignificant_drop.rs | 130 +++++++++++++++++
.../migrations/insignificant_drop.stderr | 105 ++++++++++++++
.../migrations/no_migrations.rs | 84 +++++++++++
.../migrations/significant_drop.rs | 137 ++++++++++++++++++
.../migrations/significant_drop.stderr | 103 +++++++++++++
5 files changed, 559 insertions(+)
create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs
create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
new file mode 100644
index 0000000000000..37fab71be45df
--- /dev/null
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
@@ -0,0 +1,130 @@
+#![deny(disjoint_capture_drop_reorder)]
+//~^ NOTE: the lint level is defined here
+
+// Test cases for types that implement a insignificant drop (stlib defined)
+
+// `t` needs Drop because one of its elements needs drop,
+// therefore precise capture might affect drop ordering
+fn test1_all_need_migration() {
+ let t = (String::new(), String::new());
+ let t1 = (String::new(), String::new());
+ let t2 = (String::new(), String::new());
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t, t1, t2) = (t, t1, t2);
+ let _t = t.0;
+ let _t1 = t1.0;
+ let _t2 = t2.0;
+ };
+
+ c();
+}
+
+// String implements drop and therefore should be migrated.
+// But in this test cases, `t2` is completely captured and when it is dropped won't be affected
+fn test2_only_precise_paths_need_migration() {
+ let t = (String::new(), String::new());
+ let t1 = (String::new(), String::new());
+ let t2 = (String::new(), String::new());
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t, t1) = (t, t1);
+ let _t = t.0;
+ let _t1 = t1.0;
+ let _t2 = t2;
+ };
+
+ c();
+}
+
+// If a variable would've not been captured by value then it would've not been
+// dropped with the closure and therefore doesn't need migration.
+fn test3_only_by_value_need_migration() {
+ let t = (String::new(), String::new());
+ let t1 = (String::new(), String::new());
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ println!("{}", t1.1);
+ };
+
+ c();
+}
+
+// Copy types get copied into the closure instead of move. Therefore we don't need to
+// migrate then as their drop order isn't tied to the closure.
+fn test4_only_non_copy_types_need_migration() {
+ let t = (String::new(), String::new());
+
+ // `t1` is Copy because all of its elements are Copy
+ let t1 = (0i32, 0i32);
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ let _t1 = t1.0;
+ };
+
+ c();
+}
+
+fn test5_only_drop_types_need_migration() {
+ struct S(i32, i32);
+
+ let t = (String::new(), String::new());
+
+ // `s` doesn't implement Drop or any elements within it, and doesn't need migration
+ let s = S(0i32, 0i32);
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ let _s = s.0;
+ };
+
+ c();
+}
+
+// Since we are using a move closure here, both `t` and `t1` get moved
+// even though they are being used by ref inside the closure.
+fn test6_move_closures_non_copy_types_might_need_migration() {
+ let t = (String::new(), String::new());
+ let t1 = (String::new(), String::new());
+ let c = move || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t1, t) = (t1, t);
+ println!("{} {}", t1.1, t.1);
+ };
+
+ c();
+}
+
+// Test migration analysis in case of Drop + Non Drop aggregates.
+// Note we need migration here only because the non-copy (because Drop type) is captured,
+// otherwise we won't need to, since we can get away with just by ref capture in that case.
+fn test7_drop_non_drop_aggregate_need_migration() {
+ let t = (String::new(), 0i32);
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ };
+
+ c();
+}
+
+fn main() {
+ test1_all_need_migration();
+ test2_only_precise_paths_need_migration();
+ test3_only_by_value_need_migration();
+ test4_only_non_copy_types_need_migration();
+ test5_only_drop_types_need_migration();
+ test6_move_closures_non_copy_types_might_need_migration();
+ test7_drop_non_drop_aggregate_need_migration();
+}
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
new file mode 100644
index 0000000000000..8b35105cf8258
--- /dev/null
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
@@ -0,0 +1,105 @@
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:13:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | let _t1 = t1.0;
+LL | | let _t2 = t2.0;
+LL | | };
+ | |_____^
+ |
+note: the lint level is defined here
+ --> $DIR/insignificant_drop.rs:1:9
+ |
+LL | #![deny(disjoint_capture_drop_reorder)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ = note: let (t, t1, t2) = (t, t1, t2);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:31:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | let _t1 = t1.0;
+LL | | let _t2 = t2;
+LL | | };
+ | |_____^
+ |
+ = note: let (t, t1) = (t, t1);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:47:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | println!("{}", t1.1);
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:65:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | let _t1 = t1.0;
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:83:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | let _s = s.0;
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:98:13
+ |
+LL | let c = move || {
+ | _____________^
+LL | |
+LL | |
+LL | | println!("{} {}", t1.1, t.1);
+LL | | };
+ | |_____^
+ |
+ = note: let (t1, t) = (t1, t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/insignificant_drop.rs:113:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: aborting due to 7 previous errors
+
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs b/src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs
new file mode 100644
index 0000000000000..73592ce04c28f
--- /dev/null
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs
@@ -0,0 +1,84 @@
+// run-pass
+
+// Set of test cases that don't need migrations
+
+#![deny(disjoint_capture_drop_reorder)]
+
+
+// Copy types as copied by the closure instead of being moved into the closure
+// Therefore their drop order isn't tied to the closure and won't be requiring any
+// migrations.
+fn test1_only_copy_types() {
+ let t = (0i32, 0i32);
+
+ let c = || {
+ let _t = t.0;
+ };
+
+ c();
+}
+
+// Same as test1 but using a move closure
+fn test2_only_copy_types_move_closure() {
+ let t = (0i32, 0i32);
+
+ let c = move || {
+ println!("{}", t.0);
+ };
+
+ c();
+}
+
+// Don't need to migrate if captured by ref
+fn test3_only_copy_types_move_closure() {
+ let t = (String::new(), String::new());
+
+ let c = || {
+ println!("{}", t.0);
+ };
+
+ c();
+}
+
+// Test migration analysis in case of Insignificant Drop + Non Drop aggregates.
+// Note in this test the closure captures a non Drop type and therefore the variable
+// is only captured by ref.
+fn test4_insignificant_drop_non_drop_aggregate() {
+ let t = (String::new(), 0i32);
+
+ let c = || {
+ let _t = t.1;
+ };
+
+ c();
+}
+
+
+struct Foo(i32);
+impl Drop for Foo {
+ fn drop(&mut self) {
+ println!("{:?} dropped", self.0);
+ }
+}
+
+// Test migration analysis in case of Significant Drop + Non Drop aggregates.
+// Note in this test the closure captures a non Drop type and therefore the variable
+// is only captured by ref.
+fn test5_significant_drop_non_drop_aggregate() {
+ let t = (Foo(0), 0i32);
+
+ let c = || {
+ let _t = t.1;
+ };
+
+ c();
+}
+
+fn main() {
+ test1_only_copy_types();
+ test2_only_copy_types_move_closure();
+ test3_only_copy_types_move_closure();
+ test4_insignificant_drop_non_drop_aggregate();
+ test5_significant_drop_non_drop_aggregate();
+
+}
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
new file mode 100644
index 0000000000000..1818e2dcc6478
--- /dev/null
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
@@ -0,0 +1,137 @@
+#![deny(disjoint_capture_drop_reorder)]
+//~^ NOTE: the lint level is defined here
+
+// Test cases for types that implement a significant drop (user defined)
+
+#[derive(Debug)]
+struct Foo(i32);
+impl Drop for Foo {
+ fn drop(&mut self) {
+ println!("{:?} dropped", self.0);
+ }
+}
+
+#[derive(Debug)]
+struct ConstainsDropField(Foo, Foo);
+
+// `t` needs Drop because one of its elements needs drop,
+// therefore precise capture might affect drop ordering
+fn test1_all_need_migration() {
+ let t = (Foo(0), Foo(0));
+ let t1 = (Foo(0), Foo(0));
+ let t2 = (Foo(0), Foo(0));
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t, t1, t2) = (t, t1, t2);
+ let _t = t.0;
+ let _t1 = t1.0;
+ let _t2 = t2.0;
+ };
+
+ c();
+}
+
+// String implements drop and therefore should be migrated.
+// But in this test cases, `t2` is completely captured and when it is dropped won't be affected
+fn test2_only_precise_paths_need_migration() {
+ let t = (Foo(0), Foo(0));
+ let t1 = (Foo(0), Foo(0));
+ let t2 = (Foo(0), Foo(0));
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t, t1) = (t, t1);
+ let _t = t.0;
+ let _t1 = t1.0;
+ let _t2 = t2;
+ };
+
+ c();
+}
+
+// If a variable would've not been captured by value then it would've not been
+// dropped with the closure and therefore doesn't need migration.
+fn test3_only_by_value_need_migration() {
+ let t = (Foo(0), Foo(0));
+ let t1 = (Foo(0), Foo(0));
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ println!("{:?}", t1.1);
+ };
+
+ c();
+}
+
+// The root variable might not implement drop themselves but some path starting
+// at the root variable might implement Drop.
+//
+// If this path isn't captured we need to migrate for the root variable.
+fn test4_type_contains_drop_need_migration() {
+ let t = ConstainsDropField(Foo(0), Foo(0));
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ };
+
+ c();
+}
+
+// Test migration analysis in case of Drop + Non Drop aggregates.
+// Note we need migration here only because the non-copy (because Drop type) is captured,
+// otherwise we won't need to, since we can get away with just by ref capture in that case.
+fn test5_drop_non_drop_aggregate_need_migration() {
+ let t = (Foo(0), 0i32);
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.0;
+ };
+
+ c();
+}
+
+// Test migration analysis in case of Significant and Insignificant Drop aggregates.
+fn test6_significant_insignificant_drop_aggregate_need_migration() {
+ struct S(i32, i32);
+
+ let t = (Foo(0), String::new());
+
+ let c = || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t) = (t);
+ let _t = t.1;
+ };
+
+ c();
+}
+
+// Since we are using a move closure here, both `t` and `t1` get moved
+// even though they are being used by ref inside the closure.
+fn test7_move_closures_non_copy_types_might_need_migration() {
+ let t = (Foo(0), Foo(0));
+ let t1 = (Foo(0), Foo(0), Foo(0));
+
+ let c = move || {
+ //~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
+ //~| NOTE: let (t1, t) = (t1, t);
+ println!("{:?} {:?}", t1.1, t.1);
+ };
+
+ c();
+}
+
+fn main() {
+ test1_all_need_migration();
+ test2_only_precise_paths_need_migration();
+ test3_only_by_value_need_migration();
+ test4_type_contains_drop_need_migration();
+ test5_drop_non_drop_aggregate_need_migration();
+ test6_significant_insignificant_drop_aggregate_need_migration();
+ test7_move_closures_non_copy_types_might_need_migration();
+}
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr
new file mode 100644
index 0000000000000..52b6a628cc0ba
--- /dev/null
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr
@@ -0,0 +1,103 @@
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:24:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | let _t1 = t1.0;
+LL | | let _t2 = t2.0;
+LL | | };
+ | |_____^
+ |
+note: the lint level is defined here
+ --> $DIR/significant_drop.rs:1:9
+ |
+LL | #![deny(disjoint_capture_drop_reorder)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ = note: let (t, t1, t2) = (t, t1, t2);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:42:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | let _t1 = t1.0;
+LL | | let _t2 = t2;
+LL | | };
+ | |_____^
+ |
+ = note: let (t, t1) = (t, t1);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:58:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | println!("{:?}", t1.1);
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:75:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:90:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.0;
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:105:13
+ |
+LL | let c = || {
+ | _____________^
+LL | |
+LL | |
+LL | | let _t = t.1;
+LL | | };
+ | |_____^
+ |
+ = note: let (t) = (t);
+
+error: drop order affected for closure because of `capture_disjoint_fields`
+ --> $DIR/significant_drop.rs:120:13
+ |
+LL | let c = move || {
+ | _____________^
+LL | |
+LL | |
+LL | | println!("{:?} {:?}", t1.1, t.1);
+LL | | };
+ | |_____^
+ |
+ = note: let (t1, t) = (t1, t);
+
+error: aborting due to 7 previous errors
+
From caf06bf5f5f3470f79362210a58c927711effd4d Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Sat, 2 Jan 2021 21:28:28 -0500
Subject: [PATCH 5/7] Mark the lint doc as compile_fail
---
compiler/rustc_lint_defs/src/builtin.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
index f1e5a4d47c787..199be00990761 100644
--- a/compiler/rustc_lint_defs/src/builtin.rs
+++ b/compiler/rustc_lint_defs/src/builtin.rs
@@ -3002,7 +3002,7 @@ declare_lint! {
///
/// ### Example
///
- /// ```rust
+ /// ```rust,compile_fail
/// # #![deny(disjoint_capture_drop_reorder)]
/// # #![allow(unused)]
/// struct FancyInteger(i32);
From 8f15cc1d88e2a4fdc41984da6178a8c2c92b1e2b Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Tue, 19 Jan 2021 03:15:36 -0500
Subject: [PATCH 6/7] PR fixup
---
compiler/rustc_typeck/src/check/upvar.rs | 85 ++++++++++---------
.../migrations/insignificant_drop.rs | 2 +-
.../migrations/significant_drop.rs | 2 +-
3 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs
index 16aaa48ce17fe..542d9a58e8aed 100644
--- a/compiler/rustc_typeck/src/check/upvar.rs
+++ b/compiler/rustc_typeck/src/check/upvar.rs
@@ -162,34 +162,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id);
if should_do_migration_analysis(self.tcx, closure_hir_id) {
- let need_migrations = self.compute_2229_migrations_first_pass(
- closure_def_id,
- span,
- capture_clause,
- body,
- self.typeck_results.borrow().closure_min_captures.get(&closure_def_id),
- );
-
- if !need_migrations.is_empty() {
- let need_migrations_hir_id =
- need_migrations.iter().map(|m| m.0).collect::<Vec<_>>();
-
- let migrations_text =
- migration_suggestion_for_2229(self.tcx, &need_migrations_hir_id);
-
- self.tcx.struct_span_lint_hir(
- lint::builtin::DISJOINT_CAPTURE_DROP_REORDER,
- closure_hir_id,
- span,
- |lint| {
- let mut diagnostics_builder = lint.build(
- "drop order affected for closure because of `capture_disjoint_fields`",
- );
- diagnostics_builder.note(&migrations_text);
- diagnostics_builder.emit();
- },
- );
- }
+ self.perform_2229_migration_anaysis(closure_def_id, capture_clause, span, body);
}
// We now fake capture information for all variables that are mentioned within the closure
@@ -555,6 +528,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
typeck_results.closure_min_captures.insert(closure_def_id, root_var_min_capture_list);
}
+ /// Perform the migration analysis for RFC 2229, and emit lint
+ /// `disjoint_capture_drop_reorder` if needed.
+ fn perform_2229_migration_anaysis(
+ &self,
+ closure_def_id: DefId,
+ capture_clause: hir::CaptureBy,
+ span: Span,
+ body: &'tcx hir::Body<'tcx>,
+ ) {
+ let need_migrations = self.compute_2229_migrations_first_pass(
+ closure_def_id,
+ span,
+ capture_clause,
+ body,
+ self.typeck_results.borrow().closure_min_captures.get(&closure_def_id),
+ );
+
+ if !need_migrations.is_empty() {
+ let need_migrations_hir_id = need_migrations.iter().map(|m| m.0).collect::<Vec<_>>();
+
+ let migrations_text = migration_suggestion_for_2229(self.tcx, &need_migrations_hir_id);
+
+ let local_def_id = closure_def_id.expect_local();
+ let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id);
+ self.tcx.struct_span_lint_hir(
+ lint::builtin::DISJOINT_CAPTURE_DROP_REORDER,
+ closure_hir_id,
+ span,
+ |lint| {
+ let mut diagnostics_builder = lint.build(
+ "drop order affected for closure because of `capture_disjoint_fields`",
+ );
+ diagnostics_builder.note(&migrations_text);
+ diagnostics_builder.emit();
+ },
+ );
+ }
+ }
+
/// Figures out the list of root variables (and their types) that aren't completely
/// captured by the closure when `capture_disjoint_fields` is enabled and drop order of
/// some path starting at that root variable **might** be affected.
@@ -617,17 +629,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let is_moved = root_var_min_capture_list
.iter()
- .find(|capture| matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue(_)))
- .is_some();
-
- // 1. If we capture more than one path starting at the root variabe then the root variable
- // isn't being captured in its entirety
- // 2. If we only capture one path starting at the root variable, it's still possible
- // that it isn't the root variable completely.
- if is_moved
- && ((root_var_min_capture_list.len() > 1)
- || (root_var_min_capture_list[0].place.projections.len() > 0))
- {
+ .any(|capture| matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue(_)));
+
+ let is_not_completely_captured =
+ root_var_min_capture_list.iter().any(|capture| capture.place.projections.len() > 0);
+
+ if is_moved && is_not_completely_captured {
need_migrations.push((var_hir_id, ty));
}
}
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
index 37fab71be45df..5b5092e9db9a4 100644
--- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
@@ -108,7 +108,7 @@ fn test6_move_closures_non_copy_types_might_need_migration() {
// Note we need migration here only because the non-copy (because Drop type) is captured,
// otherwise we won't need to, since we can get away with just by ref capture in that case.
fn test7_drop_non_drop_aggregate_need_migration() {
- let t = (String::new(), 0i32);
+ let t = (String::new(), String::new(), 0i32);
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
index 1818e2dcc6478..25b5539b86289 100644
--- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
@@ -85,7 +85,7 @@ fn test4_type_contains_drop_need_migration() {
// Note we need migration here only because the non-copy (because Drop type) is captured,
// otherwise we won't need to, since we can get away with just by ref capture in that case.
fn test5_drop_non_drop_aggregate_need_migration() {
- let t = (Foo(0), 0i32);
+ let t = (Foo(0), Foo(0), 0i32);
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
From 84f0a0a1c6627e83408d1faf46e29d29d2bc13e9 Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Tue, 26 Jan 2021 04:08:23 -0500
Subject: [PATCH 7/7] New migration
---
compiler/rustc_typeck/src/check/upvar.rs | 2 +-
.../migrations/insignificant_drop.rs | 14 +++++++-------
.../migrations/insignificant_drop.stderr | 14 +++++++-------
.../migrations/significant_drop.rs | 14 +++++++-------
.../migrations/significant_drop.stderr | 14 +++++++-------
5 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs
index 542d9a58e8aed..04a9e65e6647d 100644
--- a/compiler/rustc_typeck/src/check/upvar.rs
+++ b/compiler/rustc_typeck/src/check/upvar.rs
@@ -1270,7 +1270,7 @@ fn migration_suggestion_for_2229(tcx: TyCtxt<'_>, need_migrations: &Vec<hir::Hir
need_migrations.iter().map(|v| format!("{}", var_name(tcx, *v))).collect::<Vec<_>>();
let migrations_list_concat = need_migrations_strings.join(", ");
- format!("let ({}) = ({});", migrations_list_concat, migrations_list_concat)
+ format!("drop(&({}));", migrations_list_concat)
}
/// Helper function to determine if we need to escalate CaptureKind from
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
index 5b5092e9db9a4..02b373620966e 100644
--- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
@@ -12,7 +12,7 @@ fn test1_all_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t, t1, t2) = (t, t1, t2);
+ //~| NOTE: drop(&(t, t1, t2));
let _t = t.0;
let _t1 = t1.0;
let _t2 = t2.0;
@@ -30,7 +30,7 @@ fn test2_only_precise_paths_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t, t1) = (t, t1);
+ //~| NOTE: drop(&(t, t1));
let _t = t.0;
let _t1 = t1.0;
let _t2 = t2;
@@ -46,7 +46,7 @@ fn test3_only_by_value_need_migration() {
let t1 = (String::new(), String::new());
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
println!("{}", t1.1);
};
@@ -64,7 +64,7 @@ fn test4_only_non_copy_types_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
let _t1 = t1.0;
};
@@ -82,7 +82,7 @@ fn test5_only_drop_types_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
let _s = s.0;
};
@@ -97,7 +97,7 @@ fn test6_move_closures_non_copy_types_might_need_migration() {
let t1 = (String::new(), String::new());
let c = move || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t1, t) = (t1, t);
+ //~| NOTE: drop(&(t1, t));
println!("{} {}", t1.1, t.1);
};
@@ -112,7 +112,7 @@ fn test7_drop_non_drop_aggregate_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
};
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
index 8b35105cf8258..656c132c12dee 100644
--- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
@@ -16,7 +16,7 @@ note: the lint level is defined here
|
LL | #![deny(disjoint_capture_drop_reorder)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- = note: let (t, t1, t2) = (t, t1, t2);
+ = note: drop(&(t, t1, t2));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop.rs:31:13
@@ -31,7 +31,7 @@ LL | | let _t2 = t2;
LL | | };
| |_____^
|
- = note: let (t, t1) = (t, t1);
+ = note: drop(&(t, t1));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop.rs:47:13
@@ -45,7 +45,7 @@ LL | | println!("{}", t1.1);
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop.rs:65:13
@@ -59,7 +59,7 @@ LL | | let _t1 = t1.0;
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop.rs:83:13
@@ -73,7 +73,7 @@ LL | | let _s = s.0;
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop.rs:98:13
@@ -86,7 +86,7 @@ LL | | println!("{} {}", t1.1, t.1);
LL | | };
| |_____^
|
- = note: let (t1, t) = (t1, t);
+ = note: drop(&(t1, t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop.rs:113:13
@@ -99,7 +99,7 @@ LL | | let _t = t.0;
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: aborting due to 7 previous errors
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
index 25b5539b86289..ed5e4ea8be011 100644
--- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
@@ -23,7 +23,7 @@ fn test1_all_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t, t1, t2) = (t, t1, t2);
+ //~| NOTE: drop(&(t, t1, t2));
let _t = t.0;
let _t1 = t1.0;
let _t2 = t2.0;
@@ -41,7 +41,7 @@ fn test2_only_precise_paths_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t, t1) = (t, t1);
+ //~| NOTE: drop(&(t, t1));
let _t = t.0;
let _t1 = t1.0;
let _t2 = t2;
@@ -57,7 +57,7 @@ fn test3_only_by_value_need_migration() {
let t1 = (Foo(0), Foo(0));
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
println!("{:?}", t1.1);
};
@@ -74,7 +74,7 @@ fn test4_type_contains_drop_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
};
@@ -89,7 +89,7 @@ fn test5_drop_non_drop_aggregate_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.0;
};
@@ -104,7 +104,7 @@ fn test6_significant_insignificant_drop_aggregate_need_migration() {
let c = || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t) = (t);
+ //~| NOTE: drop(&(t));
let _t = t.1;
};
@@ -119,7 +119,7 @@ fn test7_move_closures_non_copy_types_might_need_migration() {
let c = move || {
//~^ERROR: drop order affected for closure because of `capture_disjoint_fields`
- //~| NOTE: let (t1, t) = (t1, t);
+ //~| NOTE: drop(&(t1, t));
println!("{:?} {:?}", t1.1, t.1);
};
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr
index 52b6a628cc0ba..6c21b27b493ba 100644
--- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr
+++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr
@@ -16,7 +16,7 @@ note: the lint level is defined here
|
LL | #![deny(disjoint_capture_drop_reorder)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- = note: let (t, t1, t2) = (t, t1, t2);
+ = note: drop(&(t, t1, t2));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/significant_drop.rs:42:13
@@ -31,7 +31,7 @@ LL | | let _t2 = t2;
LL | | };
| |_____^
|
- = note: let (t, t1) = (t, t1);
+ = note: drop(&(t, t1));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/significant_drop.rs:58:13
@@ -45,7 +45,7 @@ LL | | println!("{:?}", t1.1);
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/significant_drop.rs:75:13
@@ -58,7 +58,7 @@ LL | | let _t = t.0;
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/significant_drop.rs:90:13
@@ -71,7 +71,7 @@ LL | | let _t = t.0;
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/significant_drop.rs:105:13
@@ -84,7 +84,7 @@ LL | | let _t = t.1;
LL | | };
| |_____^
|
- = note: let (t) = (t);
+ = note: drop(&(t));
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/significant_drop.rs:120:13
@@ -97,7 +97,7 @@ LL | | println!("{:?} {:?}", t1.1, t.1);
LL | | };
| |_____^
|
- = note: let (t1, t) = (t1, t);
+ = note: drop(&(t1, t));
error: aborting due to 7 previous errors