Skip to content

Commit 4c7bb8b

Browse files
borsbors
committed
Auto merge of #61276 - eddyb:kill-res-upvar, r=petrochenkov
rustc: remove Res::Upvar. By keeping track of the current "`body_owner`" (the `DefId` of the current fn/closure/const/etc.) in several passes, `Res::Upvar` and `hir::Upvar` don't need to contain contextual information about the closure. By leveraging [`indexmap`](https://docs.rs/indexmap), the list of upvars for a given closure can now also be queried, to check whether a local variable is a closure capture, and so `Res::Upvar` can be merged with `Res::Local`. And finally, the `tcx.upvars(...)` query now collects upvars from HIR, without relying on `rustc_resolve`. r? @petrochenkov cc @varkor @davidtwco
2 parents 538e17a + f7a4c9d commit 4c7bb8b

File tree

46 files changed

+458
-292
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+458
-292
lines changed

‎Cargo.lock

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,11 @@ dependencies = [
11831183
"typenum 1.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
11841184
]
11851185

1186+
[[package]]
1187+
name = "indexmap"
1188+
version = "1.0.2"
1189+
source = "registry+https://github.com/rust-lang/crates.io-index"
1190+
11861191
[[package]]
11871192
name = "installer"
11881193
version = "0.0.0"
@@ -2719,6 +2724,7 @@ dependencies = [
27192724
"cfg-if 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
27202725
"ena 0.13.0 (registry+https://github.com/rust-lang/crates.io-index)",
27212726
"graphviz 0.0.0",
2727+
"indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
27222728
"jobserver 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)",
27232729
"lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
27242730
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2970,6 +2976,7 @@ version = "0.0.0"
29702976
dependencies = [
29712977
"arena 0.0.0",
29722978
"bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
2979+
"indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
29732980
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
29742981
"rustc 0.0.0",
29752982
"rustc_data_structures 0.0.0",
@@ -3236,6 +3243,7 @@ dependencies = [
32363243
name = "serialize"
32373244
version = "0.0.0"
32383245
dependencies = [
3246+
"indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
32393247
"smallvec 0.6.7 (registry+https://github.com/rust-lang/crates.io-index)",
32403248
]
32413249

@@ -4199,6 +4207,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
41994207
"checksum if_chain 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c3360c7b59e5ffa2653671fb74b4741a5d343c03f331c0a4aeda42b5c2b0ec7d"
42004208
"checksum ignore 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "8dc57fa12805f367736a38541ac1a9fc6a52812a0ca959b1d4d4b640a89eb002"
42014209
"checksum im-rc 13.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "0a0197597d095c0d11107975d3175173f810ee572c2501ff4de64f4f3f119806"
4210+
"checksum indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7e81a7c05f79578dbc15793d8b619db9ba32b4577003ef3af1a91c416798c58d"
42024211
"checksum iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe6e417e7d0975db6512b90796e8ce223145ac4e33c377e4a42882a0e88bb08"
42034212
"checksum is-match 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7e5b386aef33a1c677be65237cb9d32c3f3ef56bd035949710c4bb13083eb053"
42044213
"checksum itertools 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)" = "f58856976b776fedd95533137617a02fb25719f40e7d9b01c7043cd65474f450"

‎src/librustc/arena.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ macro_rules! arena_types {
109109
>,
110110
[few] crate_variances: rustc::ty::CrateVariancesMap<'tcx>,
111111
[few] inferred_outlives_crate: rustc::ty::CratePredicatesMap<'tcx>,
112+
[] upvars: rustc_data_structures::fx::FxIndexMap<rustc::hir::HirId, rustc::hir::Upvar>,
112113
], $tcx);
113114
)
114115
}

‎src/librustc/hir/def.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,6 @@ pub enum Res<Id = hir::HirId> {
139139
// Value namespace
140140
SelfCtor(DefId /* impl */), // `DefId` refers to the impl
141141
Local(Id),
142-
Upvar(Id, // `HirId` of closed over local
143-
usize, // index in the `upvars` list of the closure
144-
ast::NodeId), // expr node that creates the closure
145142

146143
// Macro namespace
147144
NonMacroAttr(NonMacroAttrKind), // e.g., `#[inline]` or `#[rustfmt::skip]`
@@ -347,7 +344,6 @@ impl<Id> Res<Id> {
347344
Res::Def(_, id) => Some(id),
348345

349346
Res::Local(..) |
350-
Res::Upvar(..) |
351347
Res::PrimTy(..) |
352348
Res::SelfTy(..) |
353349
Res::SelfCtor(..) |
@@ -374,7 +370,6 @@ impl<Id> Res<Id> {
374370
Res::SelfCtor(..) => "self constructor",
375371
Res::PrimTy(..) => "builtin type",
376372
Res::Local(..) => "local variable",
377-
Res::Upvar(..) => "closure capture",
378373
Res::SelfTy(..) => "self type",
379374
Res::ToolMod => "tool module",
380375
Res::NonMacroAttr(attr_kind) => attr_kind.descr(),
@@ -397,11 +392,6 @@ impl<Id> Res<Id> {
397392
Res::SelfCtor(id) => Res::SelfCtor(id),
398393
Res::PrimTy(id) => Res::PrimTy(id),
399394
Res::Local(id) => Res::Local(map(id)),
400-
Res::Upvar(id, index, closure) => Res::Upvar(
401-
map(id),
402-
index,
403-
closure
404-
),
405395
Res::SelfTy(a, b) => Res::SelfTy(a, b),
406396
Res::ToolMod => Res::ToolMod,
407397
Res::NonMacroAttr(attr_kind) => Res::NonMacroAttr(attr_kind),

‎src/librustc/hir/map/mod.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::util::common::time;
2626

2727
use std::io;
2828
use std::result::Result::Err;
29-
use crate::ty::TyCtxt;
29+
use crate::ty::query::Providers;
3030

3131
pub mod blocks;
3232
mod collector;
@@ -1450,11 +1450,13 @@ fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String {
14501450
}
14511451
}
14521452

1453-
pub fn def_kind(tcx: TyCtxt<'_, '_, '_>, def_id: DefId) -> Option<DefKind> {
1454-
if let Some(node_id) = tcx.hir().as_local_node_id(def_id) {
1455-
tcx.hir().def_kind(node_id)
1456-
} else {
1457-
bug!("Calling local def_kind query provider for upstream DefId: {:?}",
1458-
def_id)
1459-
}
1453+
pub fn provide(providers: &mut Providers<'_>) {
1454+
providers.def_kind = |tcx, def_id| {
1455+
if let Some(node_id) = tcx.hir().as_local_node_id(def_id) {
1456+
tcx.hir().def_kind(node_id)
1457+
} else {
1458+
bug!("Calling local def_kind query provider for upstream DefId: {:?}",
1459+
def_id)
1460+
}
1461+
};
14601462
}

‎src/librustc/hir/mod.rs

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ pub mod lowering;
6363
pub mod map;
6464
pub mod pat_util;
6565
pub mod print;
66+
pub mod upvars;
6667

6768
/// Uniquely identifies a node in the HIR of the current crate. It is
6869
/// composed of the `owner`, which is the `DefIndex` of the directly enclosing
@@ -1408,7 +1409,6 @@ impl Expr {
14081409
ExprKind::Path(QPath::Resolved(_, ref path)) => {
14091410
match path.res {
14101411
Res::Local(..)
1411-
| Res::Upvar(..)
14121412
| Res::Def(DefKind::Static, _)
14131413
| Res::Err => true,
14141414
_ => false,
@@ -2493,32 +2493,11 @@ impl ForeignItemKind {
24932493

24942494
/// A variable captured by a closure.
24952495
#[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)]
2496-
pub struct Upvar<Id = HirId> {
2497-
/// The variable being captured.
2498-
pub res: Res<Id>,
2499-
2496+
pub struct Upvar {
25002497
// First span where it is accessed (there can be multiple).
25012498
pub span: Span
25022499
}
25032500

2504-
impl<Id: fmt::Debug + Copy> Upvar<Id> {
2505-
pub fn map_id<R>(self, map: impl FnMut(Id) -> R) -> Upvar<R> {
2506-
Upvar {
2507-
res: self.res.map_id(map),
2508-
span: self.span,
2509-
}
2510-
}
2511-
2512-
pub fn var_id(&self) -> Id {
2513-
match self.res {
2514-
Res::Local(id) | Res::Upvar(id, ..) => id,
2515-
_ => bug!("Upvar::var_id: bad res ({:?})", self.res)
2516-
}
2517-
}
2518-
}
2519-
2520-
pub type UpvarMap = NodeMap<Vec<Upvar<ast::NodeId>>>;
2521-
25222501
pub type CaptureModeMap = NodeMap<CaptureClause>;
25232502

25242503
// The TraitCandidate's import_ids is empty if the trait is defined in the same module, and
@@ -2537,10 +2516,10 @@ pub type TraitMap = NodeMap<Vec<TraitCandidate>>;
25372516
// imported.
25382517
pub type GlobMap = NodeMap<FxHashSet<Name>>;
25392518

2540-
25412519
pub fn provide(providers: &mut Providers<'_>) {
25422520
check_attr::provide(providers);
2543-
providers.def_kind = map::def_kind;
2521+
map::provide(providers);
2522+
upvars::provide(providers);
25442523
}
25452524

25462525
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)]

‎src/librustc/hir/upvars.rs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
//! Upvar (closure capture) collection from cross-body HIR uses of `Res::Local`s.
2+
3+
use crate::hir::{self, HirId};
4+
use crate::hir::def::Res;
5+
use crate::hir::intravisit::{self, Visitor, NestedVisitorMap};
6+
use crate::ty::TyCtxt;
7+
use crate::ty::query::Providers;
8+
use syntax_pos::Span;
9+
use rustc_data_structures::fx::{FxIndexMap, FxHashSet};
10+
11+
pub fn provide(providers: &mut Providers<'_>) {
12+
providers.upvars = |tcx, def_id| {
13+
if !tcx.is_closure(def_id) {
14+
return None;
15+
}
16+
17+
let node_id = tcx.hir().as_local_node_id(def_id).unwrap();
18+
let body = tcx.hir().body(tcx.hir().maybe_body_owned_by(node_id)?);
19+
20+
let mut local_collector = LocalCollector::default();
21+
local_collector.visit_body(body);
22+
23+
let mut capture_collector = CaptureCollector {
24+
tcx,
25+
locals: &local_collector.locals,
26+
upvars: FxIndexMap::default(),
27+
};
28+
capture_collector.visit_body(body);
29+
30+
if !capture_collector.upvars.is_empty() {
31+
Some(tcx.arena.alloc(capture_collector.upvars))
32+
} else {
33+
None
34+
}
35+
};
36+
}
37+
38+
#[derive(Default)]
39+
struct LocalCollector {
40+
// FIXME(eddyb) perhaps use `ItemLocalId` instead?
41+
locals: FxHashSet<HirId>,
42+
}
43+
44+
impl Visitor<'tcx> for LocalCollector {
45+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
46+
NestedVisitorMap::None
47+
}
48+
49+
fn visit_pat(&mut self, pat: &'tcx hir::Pat) {
50+
if let hir::PatKind::Binding(_, hir_id, ..) = pat.node {
51+
self.locals.insert(hir_id);
52+
}
53+
intravisit::walk_pat(self, pat);
54+
}
55+
}
56+
57+
struct CaptureCollector<'a, 'tcx> {
58+
tcx: TyCtxt<'a, 'tcx, 'tcx>,
59+
locals: &'a FxHashSet<HirId>,
60+
upvars: FxIndexMap<HirId, hir::Upvar>,
61+
}
62+
63+
impl CaptureCollector<'_, '_> {
64+
fn visit_local_use(&mut self, var_id: HirId, span: Span) {
65+
if !self.locals.contains(&var_id) {
66+
self.upvars.entry(var_id).or_insert(hir::Upvar { span });
67+
}
68+
}
69+
}
70+
71+
impl Visitor<'tcx> for CaptureCollector<'a, 'tcx> {
72+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
73+
NestedVisitorMap::None
74+
}
75+
76+
fn visit_path(&mut self, path: &'tcx hir::Path, _: hir::HirId) {
77+
if let Res::Local(var_id) = path.res {
78+
self.visit_local_use(var_id, path.span);
79+
}
80+
81+
intravisit::walk_path(self, path);
82+
}
83+
84+
fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
85+
if let hir::ExprKind::Closure(..) = expr.node {
86+
let closure_def_id = self.tcx.hir().local_def_id_from_hir_id(expr.hir_id);
87+
if let Some(upvars) = self.tcx.upvars(closure_def_id) {
88+
// Every capture of a closure expression is a local in scope,
89+
// that is moved/copied/borrowed into the closure value, and
90+
// for this analysis they are like any other access to a local.
91+
//
92+
// E.g. in `|b| |c| (a, b, c)`, the upvars of the inner closure
93+
// are `a` and `b`, and while `a` is not directly used in the
94+
// outer closure, it needs to be an upvar there too, so that
95+
// the inner closure can take it (from the outer closure's env).
96+
for (&var_id, upvar) in upvars {
97+
self.visit_local_use(var_id, upvar.span);
98+
}
99+
}
100+
}
101+
102+
intravisit::walk_expr(self, expr);
103+
}
104+
}

‎src/librustc/middle/dead.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
7878
}
7979
_ if self.in_pat => {},
8080
Res::PrimTy(..) | Res::SelfTy(..) | Res::SelfCtor(..) |
81-
Res::Local(..) | Res::Upvar(..) => {}
81+
Res::Local(..) => {}
8282
Res::Def(DefKind::Ctor(CtorOf::Variant, ..), ctor_def_id) => {
8383
let variant_id = self.tcx.parent(ctor_def_id).unwrap();
8484
let enum_id = self.tcx.parent(variant_id).unwrap();

‎src/librustc/middle/expr_use_visitor.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx, 'tcx> {
268268
/// See also `with_infer`, which is used *during* typeck.
269269
pub fn new(delegate: &'a mut (dyn Delegate<'tcx>+'a),
270270
tcx: TyCtxt<'a, 'tcx, 'tcx>,
271+
body_owner: DefId,
271272
param_env: ty::ParamEnv<'tcx>,
272273
region_scope_tree: &'a region::ScopeTree,
273274
tables: &'a ty::TypeckTables<'tcx>,
@@ -276,6 +277,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx, 'tcx> {
276277
{
277278
ExprUseVisitor {
278279
mc: mc::MemCategorizationContext::new(tcx,
280+
body_owner,
279281
region_scope_tree,
280282
tables,
281283
rvalue_promotable_map),
@@ -288,13 +290,19 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx, 'tcx> {
288290
impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
289291
pub fn with_infer(delegate: &'a mut (dyn Delegate<'tcx>+'a),
290292
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
293+
body_owner: DefId,
291294
param_env: ty::ParamEnv<'tcx>,
292295
region_scope_tree: &'a region::ScopeTree,
293296
tables: &'a ty::TypeckTables<'tcx>)
294297
-> Self
295298
{
296299
ExprUseVisitor {
297-
mc: mc::MemCategorizationContext::with_infer(infcx, region_scope_tree, tables),
300+
mc: mc::MemCategorizationContext::with_infer(
301+
infcx,
302+
body_owner,
303+
region_scope_tree,
304+
tables,
305+
),
298306
delegate,
299307
param_env,
300308
}
@@ -924,16 +932,15 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
924932

925933
let closure_def_id = self.tcx().hir().local_def_id_from_hir_id(closure_expr.hir_id);
926934
if let Some(upvars) = self.tcx().upvars(closure_def_id) {
927-
for upvar in upvars.iter() {
928-
let var_hir_id = upvar.var_id();
935+
for (&var_id, upvar) in upvars.iter() {
929936
let upvar_id = ty::UpvarId {
930-
var_path: ty::UpvarPath { hir_id: var_hir_id },
937+
var_path: ty::UpvarPath { hir_id: var_id },
931938
closure_expr_id: closure_def_id.to_local(),
932939
};
933940
let upvar_capture = self.mc.tables.upvar_capture(upvar_id);
934941
let cmt_var = return_if_err!(self.cat_captured_var(closure_expr.hir_id,
935942
fn_decl_span,
936-
upvar));
943+
var_id));
937944
match upvar_capture {
938945
ty::UpvarCapture::ByValue => {
939946
let mode = copy_or_move(&self.mc,
@@ -958,13 +965,12 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
958965
fn cat_captured_var(&mut self,
959966
closure_hir_id: hir::HirId,
960967
closure_span: Span,
961-
upvar: &hir::Upvar)
968+
var_id: hir::HirId)
962969
-> mc::McResult<mc::cmt_<'tcx>> {
963970
// Create the cmt for the variable being borrowed, from the
964-
// caller's perspective
965-
let var_hir_id = upvar.var_id();
966-
let var_ty = self.mc.node_ty(var_hir_id)?;
967-
self.mc.cat_res(closure_hir_id, closure_span, var_ty, upvar.res)
971+
// perspective of the creator (parent) of the closure.
972+
let var_ty = self.mc.node_ty(var_id)?;
973+
self.mc.cat_res(closure_hir_id, closure_span, var_ty, Res::Local(var_id))
968974
}
969975
}
970976

0 commit comments

Comments
 (0)