diff --git a/compiler/rustc_middle/src/ty/closure.rs b/compiler/rustc_middle/src/ty/closure.rs index 76ac6c4e08e91..c31a882c272a4 100644 --- a/compiler/rustc_middle/src/ty/closure.rs +++ b/compiler/rustc_middle/src/ty/closure.rs @@ -168,6 +168,56 @@ impl CapturedPlace<'tcx> { base => bug!("Expected upvar, found={:?}", base), } } + + /// Returns the `LocalDefId` of the closure that captureed this Place + pub fn get_closure_local_def_id(&self) -> LocalDefId { + match self.place.base { + HirPlaceBase::Upvar(upvar_id) => upvar_id.closure_expr_id, + base => bug!("expected upvar, found={:?}", base), + } + } + + /// Return span pointing to use that resulted in selecting the current capture kind + pub fn get_capture_kind_span(&self, tcx: TyCtxt<'tcx>) -> Span { + if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id { + tcx.hir().span(capture_kind_expr_id) + } else if let Some(path_expr_id) = self.info.path_expr_id { + tcx.hir().span(path_expr_id) + } else { + // Fallback on upvars mentioned if neither path or capture expr id is captured + + // Safe to unwrap since we know this place is captured by the closure, therefore the closure must have upvars. + tcx.upvars_mentioned(self.get_closure_local_def_id()).unwrap() + [&self.get_root_variable()] + .span + } + } +} + +/// Return true if the `proj_possible_ancestor` represents an ancestor path +/// to `proj_capture` or `proj_possible_ancestor` is same as `proj_capture`, +/// assuming they both start off of the same root variable. +/// +/// **Note:** It's the caller's responsibility to ensure that both lists of projections +/// start off of the same root variable. +/// +/// Eg: 1. `foo.x` which is represented using `projections=[Field(x)]` is an ancestor of +/// `foo.x.y` which is represented using `projections=[Field(x), Field(y)]`. +/// Note both `foo.x` and `foo.x.y` start off of the same root variable `foo`. +/// 2. Since we only look at the projections here function will return `bar.x` as an a valid +/// ancestor of `foo.x.y`. It's the caller's responsibility to ensure that both projections +/// list are being applied to the same root variable. +pub fn is_ancestor_or_same_capture( + proj_possible_ancestor: &[HirProjectionKind], + proj_capture: &[HirProjectionKind], +) -> bool { + // We want to make sure `is_ancestor_or_same_capture("x.0.0", "x.0")` to return false. + // Therefore we can't just check if all projections are same in the zipped iterator below. + if proj_possible_ancestor.len() > proj_capture.len() { + return false; + } + + proj_possible_ancestor.iter().zip(proj_capture).all(|(a, b)| a == b) } /// Part of `MinCaptureInformationMap`; describes the capture kind (&, &mut, move) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 9c0048d96da24..41a8bc10c8de2 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -30,9 +30,7 @@ use rustc_attr as attr; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::profiling::SelfProfilerRef; use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap}; -use rustc_data_structures::stable_hasher::{ - hash_stable_hashmap, HashStable, StableHasher, StableVec, -}; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableVec}; use rustc_data_structures::steal::Steal; use rustc_data_structures::sync::{self, Lock, Lrc, WorkerLocal}; use rustc_errors::ErrorReported; @@ -386,9 +384,6 @@ pub struct TypeckResults<'tcx> { /// pat_adjustments: ItemLocalMap>>, - /// Borrows - pub upvar_capture_map: ty::UpvarCaptureMap<'tcx>, - /// Records the reasons that we picked the kind of each closure; /// not all closures are present in the map. closure_kind_origins: ItemLocalMap<(Span, HirPlace<'tcx>)>, @@ -424,12 +419,6 @@ pub struct TypeckResults<'tcx> { /// by this function. pub concrete_opaque_types: FxHashMap>, - /// Given the closure ID this map provides the list of UpvarIDs used by it. - /// The upvarID contains the HIR node ID and it also contains the full path - /// leading to the member of the struct or tuple that is used instead of the - /// entire variable. - pub closure_captures: ty::UpvarListMap, - /// Tracks the minimum captures required for a closure; /// see `MinCaptureInformationMap` for more details. pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>, @@ -482,7 +471,6 @@ impl<'tcx> TypeckResults<'tcx> { adjustments: Default::default(), pat_binding_modes: Default::default(), pat_adjustments: Default::default(), - upvar_capture_map: Default::default(), closure_kind_origins: Default::default(), liberated_fn_sigs: Default::default(), fru_field_types: Default::default(), @@ -490,7 +478,6 @@ impl<'tcx> TypeckResults<'tcx> { used_trait_imports: Lrc::new(Default::default()), tainted_by_errors: None, concrete_opaque_types: Default::default(), - closure_captures: Default::default(), closure_min_captures: Default::default(), closure_fake_reads: Default::default(), generator_interior_types: ty::Binder::dummy(Default::default()), @@ -675,10 +662,6 @@ impl<'tcx> TypeckResults<'tcx> { .flatten() } - pub fn upvar_capture(&self, upvar_id: ty::UpvarId) -> ty::UpvarCapture<'tcx> { - self.upvar_capture_map[&upvar_id] - } - pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, HirPlace<'tcx>)> { LocalTableInContext { hir_owner: self.hir_owner, data: &self.closure_kind_origins } } @@ -732,17 +715,13 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { ref adjustments, ref pat_binding_modes, ref pat_adjustments, - ref upvar_capture_map, ref closure_kind_origins, ref liberated_fn_sigs, ref fru_field_types, - ref coercion_casts, - ref used_trait_imports, tainted_by_errors, ref concrete_opaque_types, - ref closure_captures, ref closure_min_captures, ref closure_fake_reads, ref generator_interior_types, @@ -750,6 +729,8 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { } = *self; hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { + hcx.local_def_path_hash(hir_owner); + type_dependent_defs.hash_stable(hcx, hasher); field_indices.hash_stable(hcx, hasher); user_provided_types.hash_stable(hcx, hasher); @@ -759,17 +740,6 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { adjustments.hash_stable(hcx, hasher); pat_binding_modes.hash_stable(hcx, hasher); pat_adjustments.hash_stable(hcx, hasher); - hash_stable_hashmap(hcx, hasher, upvar_capture_map, |up_var_id, hcx| { - let ty::UpvarId { var_path, closure_expr_id } = *up_var_id; - - assert_eq!(var_path.hir_id.owner, hir_owner); - - ( - hcx.local_def_path_hash(var_path.hir_id.owner), - var_path.hir_id.local_id, - hcx.local_def_path_hash(closure_expr_id), - ) - }); closure_kind_origins.hash_stable(hcx, hasher); liberated_fn_sigs.hash_stable(hcx, hasher); @@ -778,7 +748,6 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { used_trait_imports.hash_stable(hcx, hasher); tainted_by_errors.hash_stable(hcx, hasher); concrete_opaque_types.hash_stable(hcx, hasher); - closure_captures.hash_stable(hcx, hasher); closure_min_captures.hash_stable(hcx, hasher); closure_fake_reads.hash_stable(hcx, hasher); generator_interior_types.hash_stable(hcx, hasher); diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs index bc15b3972c9fc..ec561fa38580c 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs @@ -388,10 +388,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // so it's safe to call `expect_local`. // // We know the field exists so it's safe to call operator[] and `unwrap` here. - let (&var_id, _) = - self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id] - .get_index(field.index()) - .unwrap(); + let var_id = self + .infcx + .tcx + .typeck(def_id.expect_local()) + .closure_min_captures_flattened(def_id) + .nth(field.index()) + .unwrap() + .get_root_variable(); self.infcx.tcx.hir().name(var_id).to_string() } @@ -966,12 +970,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind; debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr); if let hir::ExprKind::Closure(.., body_id, args_span, _) = expr { - for (upvar_hir_id, place) in - self.infcx.tcx.typeck(def_id.expect_local()).closure_captures[&def_id] - .keys() - .zip(places) + for (captured_place, place) in self + .infcx + .tcx + .typeck(def_id.expect_local()) + .closure_min_captures_flattened(def_id) + .zip(places) { - let span = self.infcx.tcx.upvars_mentioned(local_did)?[upvar_hir_id].span; + let upvar_hir_id = captured_place.get_root_variable(); + //FIXME(project-rfc-2229#8): Use better span from captured_place + let span = self.infcx.tcx.upvars_mentioned(local_did)?[&upvar_hir_id].span; match place { Operand::Copy(place) | Operand::Move(place) if target_place == place.as_ref() => @@ -979,10 +987,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { debug!("closure_span: found captured local {:?}", place); let body = self.infcx.tcx.hir().body(*body_id); let generator_kind = body.generator_kind(); - let upvar_id = ty::UpvarId { - var_path: ty::UpvarPath { hir_id: *upvar_hir_id }, - closure_expr_id: local_did, - }; // If we have a more specific span available, point to that. // We do this even though this span might be part of a borrow error @@ -990,11 +994,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // to a span that shows why the upvar is used in the closure, // so a move-related span is as good as any (and potentially better, // if the overall error is due to a move of the upvar). - let usage_span = - match self.infcx.tcx.typeck(local_did).upvar_capture(upvar_id) { - ty::UpvarCapture::ByValue(Some(span)) => span, - _ => span, - }; + + let usage_span = match captured_place.info.capture_kind { + ty::UpvarCapture::ByValue(Some(span)) => span, + _ => span, + }; return Some((*args_span, generator_kind, usage_span)); } _ => {} diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 2f40a90fb5516..28f6508cab2da 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -510,24 +510,54 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { the_place_err: PlaceRef<'tcx>, err: &mut DiagnosticBuilder<'_>, ) { - let id = id.expect_local(); - let tables = tcx.typeck(id); - let hir_id = tcx.hir().local_def_id_to_hir_id(id); - if let Some((span, place)) = tables.closure_kind_origins().get(hir_id) { - let reason = if let PlaceBase::Upvar(upvar_id) = place.base { - let upvar = ty::place_to_string_for_capture(tcx, place); - match tables.upvar_capture(upvar_id) { - ty::UpvarCapture::ByRef(ty::UpvarBorrow { - kind: ty::BorrowKind::MutBorrow | ty::BorrowKind::UniqueImmBorrow, - .. - }) => { - format!("mutable borrow of `{}`", upvar) - } - ty::UpvarCapture::ByValue(_) => { - format!("possible mutation of `{}`", upvar) + let closure_local_def_id = id.expect_local(); + let tables = tcx.typeck(closure_local_def_id); + let closure_hir_id = tcx.hir().local_def_id_to_hir_id(closure_local_def_id); + if let Some((span, closure_kind_origin)) = + &tables.closure_kind_origins().get(closure_hir_id) + { + let reason = if let PlaceBase::Upvar(upvar_id) = closure_kind_origin.base { + let upvar = ty::place_to_string_for_capture(tcx, closure_kind_origin); + let root_hir_id = upvar_id.var_path.hir_id; + // we have a origin for this closure kind starting at this root variable so it's safe to unwrap here + let captured_places = tables.closure_min_captures[id].get(&root_hir_id).unwrap(); + + let origin_projection = closure_kind_origin + .projections + .iter() + .map(|proj| proj.kind) + .collect::>(); + let mut capture_reason = String::new(); + for captured_place in captured_places { + let captured_place_kinds = captured_place + .place + .projections + .iter() + .map(|proj| proj.kind) + .collect::>(); + if rustc_middle::ty::is_ancestor_or_same_capture( + &captured_place_kinds, + &origin_projection, + ) { + match captured_place.info.capture_kind { + ty::UpvarCapture::ByRef(ty::UpvarBorrow { + kind: ty::BorrowKind::MutBorrow | ty::BorrowKind::UniqueImmBorrow, + .. + }) => { + capture_reason = format!("mutable borrow of `{}`", upvar); + } + ty::UpvarCapture::ByValue(_) => { + capture_reason = format!("possible mutation of `{}`", upvar); + } + _ => bug!("upvar `{}` borrowed, but not mutably", upvar), + } + break; } - val => bug!("upvar `{}` borrowed, but not mutably: {:?}", upvar, val), } + if capture_reason.is_empty() { + bug!("upvar `{}` borrowed, but cannot find reason", upvar); + } + capture_reason } else { bug!("not an upvar") }; diff --git a/compiler/rustc_mir/src/interpret/validity.rs b/compiler/rustc_mir/src/interpret/validity.rs index 2d2799f81e392..062ef7d8b4cbf 100644 --- a/compiler/rustc_mir/src/interpret/validity.rs +++ b/compiler/rustc_mir/src/interpret/validity.rs @@ -77,7 +77,7 @@ macro_rules! throw_validation_failure { /// macro_rules! try_validation { ($e:expr, $where:expr, - $( $( $p:pat )|+ => { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )? ),+ $(,)? + $( $( $p:pat )|+ => { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )? ),+ $(,)? ) => {{ match $e { Ok(x) => x, @@ -244,17 +244,20 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' // generators and closures. ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { let mut name = None; - if let Some(def_id) = def_id.as_local() { - let tables = self.ecx.tcx.typeck(def_id); - if let Some(upvars) = tables.closure_captures.get(&def_id.to_def_id()) { + // FIXME this should be more descriptive i.e. CapturePlace instead of CapturedVar + // https://github.com/rust-lang/project-rfc-2229/issues/46 + if let Some(local_def_id) = def_id.as_local() { + let tables = self.ecx.tcx.typeck(local_def_id); + if let Some(captured_place) = + tables.closure_min_captures_flattened(*def_id).nth(field) + { // Sometimes the index is beyond the number of upvars (seen // for a generator). - if let Some((&var_hir_id, _)) = upvars.get_index(field) { - let node = self.ecx.tcx.hir().get(var_hir_id); - if let hir::Node::Binding(pat) = node { - if let hir::PatKind::Binding(_, _, ident, _) = pat.kind { - name = Some(ident.name); - } + let var_hir_id = captured_place.get_root_variable(); + let node = self.ecx.tcx.hir().get(var_hir_id); + if let hir::Node::Binding(pat) = node { + if let hir::PatKind::Binding(_, _, ident, _) = pat.kind { + name = Some(ident.name); } } } diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 4bc497f5ce29c..9f1de3349a56e 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -881,7 +881,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let hir_typeck_results = self.typeck_results; // In analyze_closure() in upvar.rs we gathered a list of upvars used by a - // indexed closure and we stored in a map called closure_captures in TypeckResults + // indexed closure and we stored in a map called closure_min_captures in TypeckResults // with the closure's DefId. Here, we run through that vec of UpvarIds for // the given closure and use the necessary information to create upvar // debuginfo and to fill `self.upvar_mutbls`. diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index a96f332374430..9aef49df7b4ff 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -95,7 +95,7 @@ use rustc_hir::{Expr, HirId, HirIdMap, HirIdSet}; use rustc_index::vec::IndexVec; use rustc_middle::hir::map::Map; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::{self, DefIdTree, TyCtxt}; +use rustc_middle::ty::{self, DefIdTree, RootVariableMinCaptureList, TyCtxt}; use rustc_session::lint; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::Span; @@ -331,7 +331,7 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> { } } - if let Some(captures) = maps.tcx.typeck(local_def_id).closure_captures.get(&def_id) { + if let Some(captures) = maps.tcx.typeck(local_def_id).closure_min_captures.get(&def_id) { for &var_hir_id in captures.keys() { let var_name = maps.tcx.hir().name(var_hir_id); maps.add_variable(Upvar(var_hir_id, var_name)); @@ -408,10 +408,10 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> { if let Some(captures) = self .tcx .typeck(closure_def_id) - .closure_captures + .closure_min_captures .get(&closure_def_id.to_def_id()) { - // If closure captures is Some, upvars_mentioned must also be Some + // If closure_min_captures is Some, upvars_mentioned must also be Some let upvars = self.tcx.upvars_mentioned(closure_def_id).unwrap(); call_caps.extend(captures.keys().map(|var_id| { let upvar = upvars[var_id]; @@ -481,11 +481,10 @@ const ACC_USE: u32 = 4; struct Liveness<'a, 'tcx> { ir: &'a mut IrMaps<'tcx>, - body_owner: LocalDefId, typeck_results: &'a ty::TypeckResults<'tcx>, param_env: ty::ParamEnv<'tcx>, upvars: Option<&'tcx FxIndexMap>, - closure_captures: Option<&'tcx FxIndexMap>, + closure_min_captures: Option<&'tcx RootVariableMinCaptureList<'tcx>>, successors: IndexVec>, rwu_table: rwu_table::RWUTable, @@ -509,8 +508,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { let typeck_results = ir.tcx.typeck(body_owner); let param_env = ir.tcx.param_env(body_owner); let upvars = ir.tcx.upvars_mentioned(body_owner); - let closure_captures = typeck_results.closure_captures.get(&body_owner.to_def_id()); - + let closure_min_captures = typeck_results.closure_min_captures.get(&body_owner.to_def_id()); let closure_ln = ir.add_live_node(ClosureNode); let exit_ln = ir.add_live_node(ExitNode); @@ -519,11 +517,10 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { Liveness { ir, - body_owner, typeck_results, param_env, upvars, - closure_captures, + closure_min_captures, successors: IndexVec::from_elem_n(None, num_live_nodes), rwu_table: rwu_table::RWUTable::new(num_live_nodes, num_vars), closure_ln, @@ -707,25 +704,27 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { // if they are live on the entry to the closure, since only the closure // itself can access them on subsequent calls. - if let Some(closure_captures) = self.closure_captures { + if let Some(closure_min_captures) = self.closure_min_captures { // Mark upvars captured by reference as used after closure exits. - // Since closure_captures is Some, upvars must exists too. - let upvars = self.upvars.unwrap(); - for (&var_hir_id, upvar_id) in closure_captures { - let upvar = upvars[&var_hir_id]; - match self.typeck_results.upvar_capture(*upvar_id) { - ty::UpvarCapture::ByRef(_) => { - let var = self.variable(var_hir_id, upvar.span); - self.acc(self.exit_ln, var, ACC_READ | ACC_USE); + 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::ByRef(_) => { + let var = self.variable( + var_hir_id, + captured_place.get_capture_kind_span(self.ir.tcx), + ); + self.acc(self.exit_ln, var, ACC_READ | ACC_USE); + } + ty::UpvarCapture::ByValue(_) => {} } - ty::UpvarCapture::ByValue(_) => {} } } } let succ = self.propagate_through_expr(&body.value, self.exit_ln); - if self.closure_captures.is_none() { + if self.closure_min_captures.is_none() { // Either not a closure, or closure without any captured variables. // No need to determine liveness of captured variables, since there // are none. @@ -1221,7 +1220,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { match path.res { Res::Local(hid) => { let in_upvars = self.upvars.map_or(false, |u| u.contains_key(&hid)); - let in_captures = self.closure_captures.map_or(false, |c| c.contains_key(&hid)); + let in_captures = self.closure_min_captures.map_or(false, |c| c.contains_key(&hid)); match (in_upvars, in_captures) { (false, _) | (true, true) => self.access_var(hir_id, hid, succ, acc, path.span), @@ -1422,52 +1421,52 @@ impl<'tcx> Liveness<'_, 'tcx> { } fn warn_about_unused_upvars(&self, entry_ln: LiveNode) { - let closure_captures = match self.closure_captures { + let closure_min_captures = match self.closure_min_captures { None => return, - Some(closure_captures) => closure_captures, + Some(closure_min_captures) => closure_min_captures, }; - // If closure_captures is Some(), upvars must be Some() too. - let upvars = self.upvars.unwrap(); - for &var_hir_id in closure_captures.keys() { - let upvar = upvars[&var_hir_id]; - let var = self.variable(var_hir_id, upvar.span); - let upvar_id = ty::UpvarId { - var_path: ty::UpvarPath { hir_id: var_hir_id }, - closure_expr_id: self.body_owner, - }; - match self.typeck_results.upvar_capture(upvar_id) { - ty::UpvarCapture::ByValue(_) => {} - ty::UpvarCapture::ByRef(..) => continue, - }; - if self.used_on_entry(entry_ln, var) { - if !self.live_on_entry(entry_ln, var) { + // If closure_min_captures is Some(), upvars must be Some() too. + 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::ByRef(..) => continue, + }; + let span = captured_place.get_capture_kind_span(self.ir.tcx); + let var = self.variable(var_hir_id, span); + if self.used_on_entry(entry_ln, var) { + if !self.live_on_entry(entry_ln, var) { + if let Some(name) = self.should_warn(var) { + self.ir.tcx.struct_span_lint_hir( + lint::builtin::UNUSED_ASSIGNMENTS, + var_hir_id, + vec![span], + |lint| { + lint.build(&format!( + "value captured by `{}` is never read", + name + )) + .help("did you mean to capture by reference instead?") + .emit(); + }, + ); + } + } + } else { if let Some(name) = self.should_warn(var) { self.ir.tcx.struct_span_lint_hir( - lint::builtin::UNUSED_ASSIGNMENTS, + lint::builtin::UNUSED_VARIABLES, var_hir_id, - vec![upvar.span], + vec![span], |lint| { - lint.build(&format!("value captured by `{}` is never read", name)) + lint.build(&format!("unused variable: `{}`", name)) .help("did you mean to capture by reference instead?") .emit(); }, ); } } - } else { - if let Some(name) = self.should_warn(var) { - self.ir.tcx.struct_span_lint_hir( - lint::builtin::UNUSED_VARIABLES, - var_hir_id, - vec![upvar.span], - |lint| { - lint.build(&format!("unused variable: `{}`", name)) - .help("did you mean to capture by reference instead?") - .emit(); - }, - ); - } } } } diff --git a/compiler/rustc_typeck/src/check/regionck.rs b/compiler/rustc_typeck/src/check/regionck.rs index 55c6420ae5e2e..8f8514cadb745 100644 --- a/compiler/rustc_typeck/src/check/regionck.rs +++ b/compiler/rustc_typeck/src/check/regionck.rs @@ -771,21 +771,39 @@ impl<'a, 'tcx> RegionCtxt<'a, 'tcx> { debug!("link_upvar_region(borrorw_region={:?}, upvar_id={:?}", borrow_region, upvar_id); // A by-reference upvar can't be borrowed for longer than the // upvar is borrowed from the environment. - match self.typeck_results.borrow().upvar_capture(upvar_id) { - ty::UpvarCapture::ByRef(upvar_borrow) => { - self.sub_regions( - infer::ReborrowUpvar(span, upvar_id), - borrow_region, - upvar_borrow.region, - ); - if let ty::ImmBorrow = upvar_borrow.kind { - debug!("link_upvar_region: capture by shared ref"); - return; + let closure_local_def_id = upvar_id.closure_expr_id; + let mut all_captures_are_imm_borrow = true; + for captured_place in self + .typeck_results + .borrow() + .closure_min_captures + .get(&closure_local_def_id.to_def_id()) + .and_then(|root_var_min_cap| root_var_min_cap.get(&upvar_id.var_path.hir_id)) + .into_iter() + .flatten() + { + match captured_place.info.capture_kind { + ty::UpvarCapture::ByRef(upvar_borrow) => { + self.sub_regions( + infer::ReborrowUpvar(span, upvar_id), + borrow_region, + upvar_borrow.region, + ); + if let ty::ImmBorrow = upvar_borrow.kind { + debug!("link_upvar_region: capture by shared ref"); + } else { + all_captures_are_imm_borrow = false; + } + } + ty::UpvarCapture::ByValue(_) => { + all_captures_are_imm_borrow = false; } } - ty::UpvarCapture::ByValue(_) => {} } - let fn_hir_id = self.tcx.hir().local_def_id_to_hir_id(upvar_id.closure_expr_id); + if all_captures_are_imm_borrow { + return; + } + let fn_hir_id = self.tcx.hir().local_def_id_to_hir_id(closure_local_def_id); let ty = self.resolve_node_type(fn_hir_id); debug!("link_upvar_region: ty={:?}", ty); diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index e79349796d226..8a4c69b5ac8f9 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -222,8 +222,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.log_closure_min_capture_info(closure_def_id, span); - self.min_captures_to_closure_captures_bridge(closure_def_id); - // Now that we've analyzed the closure, we know how each // variable is borrowed, and we know what traits the closure // implements (Fn vs FnMut etc). We now have some updates to do @@ -293,80 +291,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .collect() } - /// Bridge for closure analysis - /// ---------------------------- - /// - /// For closure with DefId `c`, the bridge converts structures required for supporting RFC 2229, - /// to structures currently used in the compiler for handling closure captures. - /// - /// For example the following structure will be converted: - /// - /// closure_min_captures - /// foo -> [ {foo.x, ImmBorrow}, {foo.y, MutBorrow} ] - /// bar -> [ {bar.z, ByValue}, {bar.q, MutBorrow} ] - /// - /// to - /// - /// 1. closure_captures - /// foo -> UpvarId(foo, c), bar -> UpvarId(bar, c) - /// - /// 2. upvar_capture_map - /// UpvarId(foo,c) -> MutBorrow, UpvarId(bar, c) -> ByValue - fn min_captures_to_closure_captures_bridge(&self, closure_def_id: DefId) { - let mut closure_captures: FxIndexMap = Default::default(); - let mut upvar_capture_map = ty::UpvarCaptureMap::default(); - - if let Some(min_captures) = - self.typeck_results.borrow().closure_min_captures.get(&closure_def_id) - { - for (var_hir_id, min_list) in min_captures.iter() { - for captured_place in min_list { - let place = &captured_place.place; - let capture_info = captured_place.info; - - let upvar_id = match place.base { - PlaceBase::Upvar(upvar_id) => upvar_id, - base => bug!("Expected upvar, found={:?}", base), - }; - - assert_eq!(upvar_id.var_path.hir_id, *var_hir_id); - assert_eq!(upvar_id.closure_expr_id, closure_def_id.expect_local()); - - closure_captures.insert(*var_hir_id, upvar_id); - - let new_capture_kind = - if let Some(capture_kind) = upvar_capture_map.get(&upvar_id) { - // upvar_capture_map only stores the UpvarCapture (CaptureKind), - // so we create a fake capture info with no expression. - let fake_capture_info = ty::CaptureInfo { - capture_kind_expr_id: None, - path_expr_id: None, - capture_kind: *capture_kind, - }; - determine_capture_info(fake_capture_info, capture_info).capture_kind - } else { - capture_info.capture_kind - }; - upvar_capture_map.insert(upvar_id, new_capture_kind); - } - } - } - debug!("For closure_def_id={:?}, closure_captures={:#?}", closure_def_id, closure_captures); - debug!( - "For closure_def_id={:?}, upvar_capture_map={:#?}", - closure_def_id, upvar_capture_map - ); - - if !closure_captures.is_empty() { - self.typeck_results - .borrow_mut() - .closure_captures - .insert(closure_def_id, closure_captures); - - self.typeck_results.borrow_mut().upvar_capture_map.extend(upvar_capture_map); - } - } - /// Analyzes the information collected by `InferBorrowKind` to compute the min number of /// Places (and corresponding capture kind) that we need to keep track of to support all /// the required captured paths. diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index d3866df44ff31..9a183ed9e629e 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -61,7 +61,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { wbcx.visit_body(body); wbcx.visit_min_capture_map(); wbcx.visit_fake_reads_map(); - wbcx.visit_upvar_capture_map(); wbcx.visit_closures(); wbcx.visit_liberated_fn_sigs(); wbcx.visit_fru_field_types(); @@ -79,9 +78,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { wbcx.typeck_results.treat_byte_string_as_slice = mem::take(&mut self.typeck_results.borrow_mut().treat_byte_string_as_slice); - wbcx.typeck_results.closure_captures = - mem::take(&mut self.typeck_results.borrow_mut().closure_captures); - if self.is_tainted_by_errors() { // FIXME(eddyb) keep track of `ErrorReported` from where the error was emitted. wbcx.typeck_results.tainted_by_errors = Some(ErrorReported); @@ -389,22 +385,6 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { self.typeck_results.closure_fake_reads = resolved_closure_fake_reads; } - fn visit_upvar_capture_map(&mut self) { - for (upvar_id, upvar_capture) in self.fcx.typeck_results.borrow().upvar_capture_map.iter() { - let new_upvar_capture = match *upvar_capture { - ty::UpvarCapture::ByValue(span) => ty::UpvarCapture::ByValue(span), - ty::UpvarCapture::ByRef(ref upvar_borrow) => { - ty::UpvarCapture::ByRef(ty::UpvarBorrow { - kind: upvar_borrow.kind, - region: self.tcx().lifetimes.re_erased, - }) - } - }; - debug!("Upvar capture for {:?} resolved to {:?}", upvar_id, new_upvar_capture); - self.typeck_results.upvar_capture_map.insert(*upvar_id, new_upvar_capture); - } - } - fn visit_closures(&mut self) { let fcx_typeck_results = self.fcx.typeck_results.borrow(); assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner); diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.rs new file mode 100644 index 0000000000000..09491f296f667 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.rs @@ -0,0 +1,91 @@ +// check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![allow(unreachable_code)] +#![warn(unused)] + +#[derive(Debug)] +struct Point { + x: i32, + y: i32, +} + +pub fn f() { + let mut a = 1; + let mut c = Point{ x:1, y:0 }; + + // Captured by value, but variable is dead on entry. + (move || { + // This will not trigger a warning for unused variable as + // c.x will be treated as a Non-tracked place + c.x = 1; + println!("{}", c.x); + a = 1; //~ WARN value captured by `a` is never read + println!("{}", a); + })(); + + // Read and written to, but never actually used. + (move || { + // This will not trigger a warning for unused variable as + // c.x will be treated as a Non-tracked place + c.x += 1; + a += 1; //~ WARN unused variable: `a` + })(); + + (move || { + println!("{}", c.x); + // Value is read by closure itself on later invocations. + // This will not trigger a warning for unused variable as + // c.x will be treated as a Non-tracked place + c.x += 1; + println!("{}", a); + a += 1; + })(); + let b = Box::new(42); + (move || { + println!("{}", c.x); + // Never read because this is FnOnce closure. + // This will not trigger a warning for unused variable as + // c.x will be treated as a Non-tracked place + c.x += 1; + println!("{}", a); + a += 1; //~ WARN value assigned to `a` is never read + drop(b); + })(); +} + +#[derive(Debug)] +struct MyStruct<'a> { + x: Option<& 'a str>, + y: i32, +} + +pub fn nested() { + let mut a : Option<& str>; + a = None; + let mut b : Option<& str>; + b = None; + let mut d = MyStruct{ x: None, y: 1}; + let mut e = MyStruct{ x: None, y: 1}; + (|| { + (|| { + // This will not trigger a warning for unused variable as + // d.x will be treated as a Non-tracked place + d.x = Some("d1"); + d.x = Some("d2"); + a = Some("d1"); //~ WARN value assigned to `a` is never read + a = Some("d2"); + })(); + (move || { + // This will not trigger a warning for unused variable as + //e.x will be treated as a Non-tracked place + e.x = Some("e1"); + e.x = Some("e2"); + b = Some("e1"); //~ WARN value assigned to `b` is never read + //~| WARN unused variable: `b` + b = Some("e2"); //~ WARN value assigned to `b` is never read + })(); + })(); +} + +fn main() {} diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.stderr new file mode 100644 index 0000000000000..81bbc4e1dc0c2 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness.stderr @@ -0,0 +1,79 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/liveness.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: value captured by `a` is never read + --> $DIR/liveness.rs:23:9 + | +LL | a = 1; + | ^ + | +note: the lint level is defined here + --> $DIR/liveness.rs:5:9 + | +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_assignments)]` implied by `#[warn(unused)]` + = help: did you mean to capture by reference instead? + +warning: unused variable: `a` + --> $DIR/liveness.rs:32:9 + | +LL | a += 1; + | ^ + | +note: the lint level is defined here + --> $DIR/liveness.rs:5:9 + | +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` + = help: did you mean to capture by reference instead? + +warning: value assigned to `a` is never read + --> $DIR/liveness.rs:52:9 + | +LL | a += 1; + | ^ + | + = help: maybe it is overwritten before being read? + +warning: value assigned to `a` is never read + --> $DIR/liveness.rs:76:13 + | +LL | a = Some("d1"); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: value assigned to `b` is never read + --> $DIR/liveness.rs:84:13 + | +LL | b = Some("e1"); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: value assigned to `b` is never read + --> $DIR/liveness.rs:86:13 + | +LL | b = Some("e2"); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: unused variable: `b` + --> $DIR/liveness.rs:84:13 + | +LL | b = Some("e1"); + | ^ + | + = help: did you mean to capture by reference instead? + +warning: 8 warnings emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness_unintentional_copy.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness_unintentional_copy.rs new file mode 100644 index 0000000000000..e2035464dfa8a --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness_unintentional_copy.rs @@ -0,0 +1,42 @@ +// check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![warn(unused)] + +#[derive(Debug)] +struct MyStruct { + a: i32, + b: i32, +} + +pub fn unintentional_copy_one() { + let mut a = 1; + let mut last = MyStruct{ a: 1, b: 1}; + let mut f = move |s| { + // This will not trigger a warning for unused variable + // as last.a will be treated as a Non-tracked place + last.a = s; + a = s; + //~^ WARN value assigned to `a` is never read + //~| WARN unused variable: `a` + }; + f(2); + f(3); + f(4); +} + +pub fn unintentional_copy_two() { + let mut a = 1; + let mut sum = MyStruct{ a: 1, b: 0}; + (1..10).for_each(move |x| { + // This will not trigger a warning for unused variable + // as sum.b will be treated as a Non-tracked place + sum.b += x; + a += x; //~ WARN unused variable: `a` + }); +} + +fn main() { + unintentional_copy_one(); + unintentional_copy_two(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness_unintentional_copy.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness_unintentional_copy.stderr new file mode 100644 index 0000000000000..35b0c22fc4fb9 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/liveness_unintentional_copy.stderr @@ -0,0 +1,47 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/liveness_unintentional_copy.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: value assigned to `a` is never read + --> $DIR/liveness_unintentional_copy.rs:19:9 + | +LL | a = s; + | ^ + | +note: the lint level is defined here + --> $DIR/liveness_unintentional_copy.rs:4:9 + | +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_assignments)]` implied by `#[warn(unused)]` + = help: maybe it is overwritten before being read? + +warning: unused variable: `a` + --> $DIR/liveness_unintentional_copy.rs:19:9 + | +LL | a = s; + | ^ + | +note: the lint level is defined here + --> $DIR/liveness_unintentional_copy.rs:4:9 + | +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` + = help: did you mean to capture by reference instead? + +warning: unused variable: `a` + --> $DIR/liveness_unintentional_copy.rs:35:9 + | +LL | a += x; + | ^ + | + = help: did you mean to capture by reference instead? + +warning: 4 warnings emitted +