diff --git a/src/librustc/arena.rs b/src/librustc/arena.rs index 4a89bf3313b8..e8c3914e695a 100644 --- a/src/librustc/arena.rs +++ b/src/librustc/arena.rs @@ -109,6 +109,7 @@ macro_rules! arena_types { >, [few] crate_variances: rustc::ty::CrateVariancesMap<'tcx>, [few] inferred_outlives_crate: rustc::ty::CratePredicatesMap<'tcx>, + [] upvars: rustc_data_structures::fx::FxIndexMap, ], $tcx); ) } diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index fd42c6f469e1..c84c18ce8a7e 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -26,7 +26,7 @@ use crate::util::common::time; use std::io; use std::result::Result::Err; -use crate::ty::TyCtxt; +use crate::ty::query::Providers; pub mod blocks; mod collector; @@ -1450,11 +1450,13 @@ fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String { } } -pub fn def_kind(tcx: TyCtxt<'_, '_, '_>, def_id: DefId) -> Option { - if let Some(node_id) = tcx.hir().as_local_node_id(def_id) { - tcx.hir().def_kind(node_id) - } else { - bug!("Calling local def_kind query provider for upstream DefId: {:?}", - def_id) - } +pub fn provide(providers: &mut Providers<'_>) { + providers.def_kind = |tcx, def_id| { + if let Some(node_id) = tcx.hir().as_local_node_id(def_id) { + tcx.hir().def_kind(node_id) + } else { + bug!("Calling local def_kind query provider for upstream DefId: {:?}", + def_id) + } + }; } diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index fa62ab15a976..f7daa7a94551 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -30,7 +30,6 @@ use syntax::util::parser::ExprPrecedence; use crate::ty::AdtKind; use crate::ty::query::Providers; -use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::sync::{par_for_each_in, Send, Sync}; use rustc_data_structures::thin_vec::ThinVec; use rustc_macros::HashStable; @@ -64,6 +63,7 @@ pub mod lowering; pub mod map; pub mod pat_util; pub mod print; +pub mod upvars; /// Uniquely identifies a node in the HIR of the current crate. It is /// composed of the `owner`, which is the `DefIndex` of the directly enclosing @@ -2498,8 +2498,6 @@ pub struct Upvar { pub span: Span } -pub type UpvarMap = NodeMap>; - pub type CaptureModeMap = NodeMap; // The TraitCandidate's import_ids is empty if the trait is defined in the same module, and @@ -2518,10 +2516,10 @@ pub type TraitMap = NodeMap>; // imported. pub type GlobMap = NodeMap>; - pub fn provide(providers: &mut Providers<'_>) { check_attr::provide(providers); - providers.def_kind = map::def_kind; + map::provide(providers); + upvars::provide(providers); } #[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] diff --git a/src/librustc/hir/upvars.rs b/src/librustc/hir/upvars.rs new file mode 100644 index 000000000000..a053deb55846 --- /dev/null +++ b/src/librustc/hir/upvars.rs @@ -0,0 +1,104 @@ +//! Upvar (closure capture) collection from cross-body HIR uses of `Res::Local`s. + +use crate::hir::{self, HirId}; +use crate::hir::def::Res; +use crate::hir::intravisit::{self, Visitor, NestedVisitorMap}; +use crate::ty::TyCtxt; +use crate::ty::query::Providers; +use syntax_pos::Span; +use rustc_data_structures::fx::{FxIndexMap, FxHashSet}; + +pub fn provide(providers: &mut Providers<'_>) { + providers.upvars = |tcx, def_id| { + if !tcx.is_closure(def_id) { + return None; + } + + let node_id = tcx.hir().as_local_node_id(def_id).unwrap(); + let body = tcx.hir().body(tcx.hir().maybe_body_owned_by(node_id)?); + + let mut local_collector = LocalCollector::default(); + local_collector.visit_body(body); + + let mut capture_collector = CaptureCollector { + tcx, + locals: &local_collector.locals, + upvars: FxIndexMap::default(), + }; + capture_collector.visit_body(body); + + if !capture_collector.upvars.is_empty() { + Some(tcx.arena.alloc(capture_collector.upvars)) + } else { + None + } + }; +} + +#[derive(Default)] +struct LocalCollector { + // FIXME(eddyb) perhaps use `ItemLocalId` instead? + locals: FxHashSet, +} + +impl Visitor<'tcx> for LocalCollector { + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } + + fn visit_pat(&mut self, pat: &'tcx hir::Pat) { + if let hir::PatKind::Binding(_, hir_id, ..) = pat.node { + self.locals.insert(hir_id); + } + intravisit::walk_pat(self, pat); + } +} + +struct CaptureCollector<'a, 'tcx> { + tcx: TyCtxt<'a, 'tcx, 'tcx>, + locals: &'a FxHashSet, + upvars: FxIndexMap, +} + +impl CaptureCollector<'_, '_> { + fn visit_local_use(&mut self, var_id: HirId, span: Span) { + if !self.locals.contains(&var_id) { + self.upvars.entry(var_id).or_insert(hir::Upvar { span }); + } + } +} + +impl Visitor<'tcx> for CaptureCollector<'a, 'tcx> { + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } + + fn visit_path(&mut self, path: &'tcx hir::Path, _: hir::HirId) { + if let Res::Local(var_id) = path.res { + self.visit_local_use(var_id, path.span); + } + + intravisit::walk_path(self, path); + } + + fn visit_expr(&mut self, expr: &'tcx hir::Expr) { + if let hir::ExprKind::Closure(..) = expr.node { + let closure_def_id = self.tcx.hir().local_def_id_from_hir_id(expr.hir_id); + if let Some(upvars) = self.tcx.upvars(closure_def_id) { + // Every capture of a closure expression is a local in scope, + // that is moved/copied/borrowed into the closure value, and + // for this analysis they are like any other access to a local. + // + // E.g. in `|b| |c| (a, b, c)`, the upvars of the inner closure + // are `a` and `b`, and while `a` is not directly used in the + // outer closure, it needs to be an upvar there too, so that + // the inner closure can take it (from the outer closure's env). + for (&var_id, upvar) in upvars { + self.visit_local_use(var_id, upvar.span); + } + } + } + + intravisit::walk_expr(self, expr); + } +} diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index f8f869b98429..b2d080a4f965 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -54,7 +54,6 @@ use rustc_data_structures::stable_hasher::{HashStable, hash_stable_hashmap, StableHasher, StableHasherResult, StableVec}; use arena::SyncDroplessArena; -use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::sync::{Lrc, Lock, WorkerLocal}; use std::any::Any; @@ -1063,11 +1062,6 @@ pub struct GlobalCtxt<'tcx> { pub queries: query::Queries<'tcx>, - // Records the captured variables referenced by every closure - // expression. Do not track deps for this, just recompute it from - // scratch every time. - upvars: FxHashMap>, - maybe_unused_trait_imports: FxHashSet, maybe_unused_extern_crates: Vec<(DefId, Span)>, /// A map of glob use to a set of names it actually imports. Currently only @@ -1298,12 +1292,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { }).collect(); (k, exports) }).collect(), - upvars: resolutions.upvars.into_iter().map(|(k, upvars)| { - let upvars: FxIndexMap<_, _> = upvars.into_iter().map(|(var_id, upvar)| { - (hir.node_to_hir_id(var_id), upvar) - }).collect(); - (hir.local_def_id(k), upvars) - }).collect(), maybe_unused_trait_imports: resolutions.maybe_unused_trait_imports .into_iter() @@ -3024,7 +3012,6 @@ pub fn provide(providers: &mut ty::query::Providers<'_>) { assert_eq!(id, LOCAL_CRATE); tcx.arena.alloc(middle::lang_items::collect(tcx)) }; - providers.upvars = |tcx, id| tcx.gcx.upvars.get(&id); providers.maybe_unused_trait_import = |tcx, id| { tcx.maybe_unused_trait_imports.contains(&id) }; diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index c0f582d5d132..e89fb53c2361 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -8,7 +8,7 @@ pub use self::BorrowKind::*; pub use self::IntVarValue::*; pub use self::fold::TypeFoldable; -use crate::hir::{map as hir_map, UpvarMap, GlobMap, TraitMap}; +use crate::hir::{map as hir_map, GlobMap, TraitMap}; use crate::hir::Node; use crate::hir::def::{Res, DefKind, CtorOf, CtorKind, ExportMap}; use crate::hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; @@ -123,7 +123,6 @@ mod sty; #[derive(Clone)] pub struct Resolutions { - pub upvars: UpvarMap, pub trait_map: TraitMap, pub maybe_unused_trait_imports: NodeSet, pub maybe_unused_extern_crates: Vec<(NodeId, Span)>, diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 4a96864dc9d6..9691d0337d7d 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -178,7 +178,6 @@ impl ExpansionResult { ExpansionResult { defs: Steal::new(resolver.definitions), resolutions: Steal::new(Resolutions { - upvars: resolver.upvars, export_map: resolver.export_map, trait_map: resolver.trait_map, glob_map: resolver.glob_map, @@ -197,7 +196,6 @@ impl ExpansionResult { ExpansionResult { defs: Steal::new(resolver.definitions.clone()), resolutions: Steal::new(Resolutions { - upvars: resolver.upvars.clone(), export_map: resolver.export_map.clone(), trait_map: resolver.trait_map.clone(), glob_map: resolver.glob_map.clone(), diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 7ab3de0475e7..21e759ccc650 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -29,7 +29,7 @@ use rustc::hir::def::{ }; use rustc::hir::def::Namespace::*; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId}; -use rustc::hir::{Upvar, UpvarMap, TraitCandidate, TraitMap, GlobMap}; +use rustc::hir::{TraitCandidate, TraitMap, GlobMap}; use rustc::ty::{self, DefIdTree}; use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap}; use rustc::{bug, span_bug}; @@ -852,7 +852,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { function_kind: FnKind<'tcx>, declaration: &'tcx FnDecl, _: Span, - node_id: NodeId) + _: NodeId) { debug!("(resolving function) entering function"); let (rib_kind, asyncness) = match function_kind { @@ -863,17 +863,14 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { FnKind::Closure(_) => // Async closures aren't resolved through `visit_fn`-- they're // processed separately - (ClosureRibKind(node_id), &IsAsync::NotAsync), + (NormalRibKind, &IsAsync::NotAsync), }; // Create a value rib for the function. self.ribs[ValueNS].push(Rib::new(rib_kind)); // Create a label rib for the function. - match rib_kind { - ClosureRibKind(_) => {} - _ => self.label_ribs.push(Rib::new(rib_kind)), - } + self.label_ribs.push(Rib::new(rib_kind)); // Add each argument to the rib. let mut bindings_list = FxHashMap::default(); @@ -900,11 +897,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { visit::walk_fn_ret_ty(self, &declaration.output); // Resolve the function body, potentially inside the body of an async closure - if let IsAsync::Async { closure_id, .. } = asyncness { - let rib_kind = ClosureRibKind(*closure_id); - self.ribs[ValueNS].push(Rib::new(rib_kind)); - } - match function_kind { FnKind::ItemFn(.., body) | FnKind::Method(.., body) => { if let IsAsync::Async { ref arguments, .. } = asyncness { @@ -927,19 +919,9 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> { } }; - // Leave the body of the async closure - if asyncness.is_async() { - self.ribs[ValueNS].pop(); - } - debug!("(resolving function) leaving function"); - match rib_kind { - ClosureRibKind(_) => {} - _ => { - self.label_ribs.pop(); - } - } + self.label_ribs.pop(); self.ribs[ValueNS].pop(); } @@ -1023,17 +1005,13 @@ enum GenericParameters<'a, 'b> { RibKind<'a>), } -/// The rib kind controls the translation of local -/// definitions (`Res::Local`) to upvars (`Res::Upvar`). +/// The rib kind restricts certain accesses, +/// e.g. to a `Res::Local` of an outer item. #[derive(Copy, Clone, Debug)] enum RibKind<'a> { - /// No translation needs to be applied. + /// No restriction needs to be applied. NormalRibKind, - /// We passed through a closure scope at the given `NodeId`. - /// Translate upvars as appropriate. - ClosureRibKind(NodeId /* func id */), - /// We passed through an impl or trait and are now in one of its /// methods or associated types. Allow references to ty params that impl or trait /// binds. Disallow any other upvars (including other ty params that are @@ -1673,7 +1651,6 @@ pub struct Resolver<'a> { /// Resolutions for labels (node IDs of their corresponding blocks or loops). label_res_map: NodeMap, - pub upvars: UpvarMap, pub export_map: ExportMap, pub trait_map: TraitMap, @@ -2036,7 +2013,6 @@ impl<'a> Resolver<'a> { partial_res_map: Default::default(), import_res_map: Default::default(), label_res_map: Default::default(), - upvars: Default::default(), export_map: FxHashMap::default(), trait_map: Default::default(), module_map, @@ -2506,9 +2482,6 @@ impl<'a> Resolver<'a> { for rib in self.label_ribs.iter().rev() { match rib.kind { NormalRibKind => {} - ClosureRibKind(_) => { - span_bug!(ident.span, "rustc_resolve: `ClosureRibKind` in `label_ribs`"); - } // If an invocation of this macro created `ident`, give up on `ident` // and switch to `ident`'s source from the macro definition. MacroDefinition(def) => { @@ -4014,7 +3987,7 @@ impl<'a> Resolver<'a> { diag); } - // Validate a local resolution (from ribs), potentially recording closure upvars. + // Validate a local resolution (from ribs). fn validate_res_from_ribs( &mut self, ns: Namespace, @@ -4045,7 +4018,7 @@ impl<'a> Resolver<'a> { } match res { - Res::Local(var_id) => { + Res::Local(_) => { use ResolutionError::*; let mut res_err = None; @@ -4055,12 +4028,6 @@ impl<'a> Resolver<'a> { ForwardTyParamBanRibKind | TyParamAsConstParamTy => { // Nothing to do. Continue. } - ClosureRibKind(function_id) => { - if record_used { - self.upvars.entry(function_id).or_default() - .entry(var_id).or_insert(Upvar { span }); - } - } ItemRibKind | FnItemRibKind | AssocItemRibKind => { // This was an attempt to access an upvar inside a // named function item. This is not allowed, so we @@ -4090,7 +4057,7 @@ impl<'a> Resolver<'a> { Res::Def(DefKind::TyParam, _) | Res::SelfTy(..) => { for rib in ribs { match rib.kind { - NormalRibKind | AssocItemRibKind | ClosureRibKind(..) | + NormalRibKind | AssocItemRibKind | ModuleRibKind(..) | MacroDefinition(..) | ForwardTyParamBanRibKind | ConstantItemRibKind | TyParamAsConstParamTy => { // Nothing to do. Continue. @@ -4470,21 +4437,14 @@ impl<'a> Resolver<'a> { visit::walk_expr(self, expr); self.current_type_ascription.pop(); } - // Resolve the body of async exprs inside the async closure to which they desugar - ExprKind::Async(_, async_closure_id, ref block) => { - let rib_kind = ClosureRibKind(async_closure_id); - self.ribs[ValueNS].push(Rib::new(rib_kind)); - self.visit_block(&block); - self.ribs[ValueNS].pop(); - } // `async |x| ...` gets desugared to `|x| future_from_generator(|| ...)`, so we need to // resolve the arguments within the proper scopes so that usages of them inside the // closure are detected as upvars rather than normal closure arg usages. ExprKind::Closure( - _, IsAsync::Async { closure_id: inner_closure_id, .. }, _, + _, IsAsync::Async { .. }, _, ref fn_decl, ref body, _span, ) => { - let rib_kind = ClosureRibKind(expr.id); + let rib_kind = NormalRibKind; self.ribs[ValueNS].push(Rib::new(rib_kind)); // Resolve arguments: let mut bindings_list = FxHashMap::default(); @@ -4497,14 +4457,11 @@ impl<'a> Resolver<'a> { // Now resolve the inner closure { - let rib_kind = ClosureRibKind(inner_closure_id); - self.ribs[ValueNS].push(Rib::new(rib_kind)); // No need to resolve arguments: the inner closure has none. // Resolve the return type: visit::walk_fn_ret_ty(self, &fn_decl.output); // Resolve the body self.visit_expr(body); - self.ribs[ValueNS].pop(); } self.ribs[ValueNS].pop(); } diff --git a/src/test/ui/borrowck/borrowck-closures-use-after-free.stderr b/src/test/ui/borrowck/borrowck-closures-use-after-free.stderr index a6dbcf36077a..f22b7da81194 100644 --- a/src/test/ui/borrowck/borrowck-closures-use-after-free.stderr +++ b/src/test/ui/borrowck/borrowck-closures-use-after-free.stderr @@ -4,7 +4,7 @@ error[E0502]: cannot borrow `*ptr` as immutable because it is also borrowed as m LL | let mut test = |foo: &Foo| { | ----------- mutable borrow occurs here LL | ptr = box Foo { x: ptr.x + 1 }; - | --- first borrow occurs due to use of `ptr` in closure + | --- first borrow occurs due to use of `ptr` in closure LL | }; LL | test(&*ptr); | ---- ^^^^^ immutable borrow occurs here diff --git a/src/test/ui/issues/issue-11192.stderr b/src/test/ui/issues/issue-11192.stderr index 2a9d913171c3..dfe7b3f6b5f9 100644 --- a/src/test/ui/issues/issue-11192.stderr +++ b/src/test/ui/issues/issue-11192.stderr @@ -5,7 +5,7 @@ LL | let mut test = |foo: &Foo| { | ----------- mutable borrow occurs here LL | println!("access {}", foo.x); LL | ptr = box Foo { x: ptr.x + 1 }; - | --- first borrow occurs due to use of `ptr` in closure + | --- first borrow occurs due to use of `ptr` in closure ... LL | test(&*ptr); | ---- ^^^^^ immutable borrow occurs here 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 135de0445a79..186f25a3c89d 100644 --- a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr +++ b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr @@ -7,11 +7,11 @@ LL | let mut closure1 = || p = &y; = note: defining type: DefId(0:14 ~ escape_upvar_nested[317d]::test[0]::{{closure}}[0]::{{closure}}[0]) with closure substs [ i16, extern "rust-call" fn(()), - &'_#1r mut &'_#2r i32, - &'_#3r i32, + &'_#1r i32, + &'_#2r mut &'_#3r i32, ] = note: number of external vids: 4 - = note: where '_#3r: '_#2r + = note: where '_#1r: '_#3r note: External requirements --> $DIR/escape-upvar-nested.rs:20:27 @@ -26,11 +26,11 @@ LL | | }; = note: defining type: DefId(0:13 ~ escape_upvar_nested[317d]::test[0]::{{closure}}[0]) with closure substs [ i16, extern "rust-call" fn(()), - &'_#1r mut &'_#2r i32, - &'_#3r i32, + &'_#1r i32, + &'_#2r mut &'_#3r i32, ] = note: number of external vids: 4 - = note: where '_#3r: '_#2r + = note: where '_#1r: '_#3r 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 8c37ab7b768c..0df2c0f69a71 100644 --- a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr +++ b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr @@ -7,11 +7,11 @@ LL | let mut closure = || p = &y; = note: defining type: DefId(0:13 ~ escape_upvar_ref[317d]::test[0]::{{closure}}[0]) with closure substs [ i16, extern "rust-call" fn(()), - &'_#1r mut &'_#2r i32, - &'_#3r i32, + &'_#1r i32, + &'_#2r mut &'_#3r i32, ] = note: number of external vids: 4 - = note: where '_#3r: '_#2r + = note: where '_#1r: '_#3r note: No external requirements --> $DIR/escape-upvar-ref.rs:17:1