Skip to content

Commit 6710eef

Browse files
committed
Refactor hir::lowering::Resolver
1 parent e6d6c37 commit 6710eef

File tree

3 files changed

+94
-99
lines changed

3 files changed

+94
-99
lines changed

src/librustc/hir/lowering.rs

Lines changed: 45 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use hir;
4444
use hir::map::Definitions;
4545
use hir::map::definitions::DefPathData;
4646
use hir::def_id::{DefIndex, DefId};
47-
use hir::def::{Def, DefMap, PathResolution};
47+
use hir::def::Def;
4848

4949
use std::collections::BTreeMap;
5050
use std::iter;
@@ -68,15 +68,30 @@ pub struct LoweringContext<'a> {
6868
// the form of a DefIndex) so that if we create a new node which introduces
6969
// a definition, then we can properly create the def id.
7070
parent_def: Cell<Option<DefIndex>>,
71-
resolver: Option<RefCell<&'a mut Resolver>>,
71+
resolver: RefCell<&'a mut Resolver>,
7272
}
7373

7474
pub trait Resolver {
75+
// Resolve a global hir path generated by the lowerer when expanding `for`, `if let`, etc.
7576
fn resolve_generated_global_path(&mut self, path: &hir::Path, is_value: bool) -> Def;
7677

77-
fn def_map(&mut self) -> &mut DefMap;
78+
// Record the resolution of a path or binding generated by the lowerer when expanding.
79+
fn record_resolution(&mut self, id: NodeId, def: Def);
80+
7881
// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
79-
fn definitions(&mut self) -> &mut Definitions;
82+
// This should only return `None` during testing.
83+
fn definitions(&mut self) -> Option<&mut Definitions>;
84+
}
85+
86+
pub struct DummyResolver;
87+
impl Resolver for DummyResolver {
88+
fn resolve_generated_global_path(&mut self, _path: &hir::Path, _is_value: bool) -> Def {
89+
Def::Err
90+
}
91+
fn record_resolution(&mut self, _id: NodeId, _def: Def) {}
92+
fn definitions(&mut self) -> Option<&mut Definitions> {
93+
None
94+
}
8095
}
8196

8297
impl<'a, 'hir> LoweringContext<'a> {
@@ -98,18 +113,7 @@ impl<'a, 'hir> LoweringContext<'a> {
98113
crate_root: crate_root,
99114
id_assigner: id_assigner,
100115
parent_def: Cell::new(None),
101-
resolver: Some(RefCell::new(resolver)),
102-
}
103-
}
104-
105-
// Only use this when you want a LoweringContext for testing and won't look
106-
// up def ids for anything created during lowering.
107-
pub fn testing_context(id_assigner: &'a NodeIdAssigner) -> LoweringContext<'a> {
108-
LoweringContext {
109-
crate_root: None,
110-
id_assigner: id_assigner,
111-
parent_def: Cell::new(None),
112-
resolver: None,
116+
resolver: RefCell::new(resolver),
113117
}
114118
}
115119

@@ -127,37 +131,17 @@ impl<'a, 'hir> LoweringContext<'a> {
127131
}
128132

129133
fn with_parent_def<T, F: FnOnce() -> T>(&self, parent_id: NodeId, f: F) -> T {
130-
if self.resolver.is_none() {
131-
// This should only be used for testing.
132-
return f();
133-
}
134-
135134
let old_def = self.parent_def.get();
136-
self.parent_def.set(Some(self.get_def(parent_id)));
135+
self.parent_def.set(match self.resolver.borrow_mut().definitions() {
136+
Some(defs) => Some(defs.opt_def_index(parent_id).unwrap()),
137+
None => old_def,
138+
});
139+
137140
let result = f();
138-
self.parent_def.set(old_def);
139141

142+
self.parent_def.set(old_def);
140143
result
141144
}
142-
143-
fn get_def(&self, id: NodeId) -> DefIndex {
144-
let mut resolver = self.resolver.as_ref().unwrap().borrow_mut();
145-
resolver.definitions().opt_def_index(id).unwrap()
146-
}
147-
148-
fn record_def(&self, id: NodeId, def: Def) {
149-
if let Some(ref resolver) = self.resolver {
150-
resolver.borrow_mut().def_map().insert(id, PathResolution { base_def: def, depth: 0 });
151-
}
152-
}
153-
154-
fn resolve_generated_global_path(&self, path: &hir::Path, is_value: bool) -> Def {
155-
if let Some(ref resolver) = self.resolver {
156-
resolver.borrow_mut().resolve_generated_global_path(path, is_value)
157-
} else {
158-
Def::Err
159-
}
160-
}
161145
}
162146

163147
pub fn lower_ident(_lctx: &LoweringContext, ident: Ident) -> hir::Ident {
@@ -1765,10 +1749,12 @@ fn expr_call(lctx: &LoweringContext,
17651749
fn expr_ident(lctx: &LoweringContext, span: Span, id: hir::Ident,
17661750
attrs: ThinAttributes, binding: NodeId) -> P<hir::Expr> {
17671751
let expr = expr(lctx, span, hir::ExprPath(None, path_ident(span, id)), attrs);
1768-
if let Some(ref resolver) = lctx.resolver {
1769-
let def_id = resolver.borrow_mut().definitions().local_def_id(binding);
1770-
lctx.record_def(expr.id, Def::Local(def_id, binding));
1771-
}
1752+
1753+
let mut resolver = lctx.resolver.borrow_mut();
1754+
let def = resolver.definitions().map(|defs| Def::Local(defs.local_def_id(binding), binding))
1755+
.unwrap_or(Def::Err);
1756+
resolver.record_resolution(expr.id, def);
1757+
17721758
expr
17731759
}
17741760

@@ -1779,9 +1765,9 @@ fn expr_mut_addr_of(lctx: &LoweringContext, span: Span, e: P<hir::Expr>,
17791765

17801766
fn expr_path(lctx: &LoweringContext, path: hir::Path,
17811767
attrs: ThinAttributes) -> P<hir::Expr> {
1782-
let def = lctx.resolve_generated_global_path(&path, true);
1768+
let def = lctx.resolver.borrow_mut().resolve_generated_global_path(&path, true);
17831769
let expr = expr(lctx, path.span, hir::ExprPath(None, path), attrs);
1784-
lctx.record_def(expr.id, def);
1770+
lctx.resolver.borrow_mut().record_resolution(expr.id, def);
17851771
expr
17861772
}
17871773

@@ -1811,9 +1797,9 @@ fn expr_struct(lctx: &LoweringContext,
18111797
fields: hir::HirVec<hir::Field>,
18121798
e: Option<P<hir::Expr>>,
18131799
attrs: ThinAttributes) -> P<hir::Expr> {
1814-
let def = lctx.resolve_generated_global_path(&path, false);
1800+
let def = lctx.resolver.borrow_mut().resolve_generated_global_path(&path, false);
18151801
let expr = expr(lctx, sp, hir::ExprStruct(path, fields, e), attrs);
1816-
lctx.record_def(expr.id, def);
1802+
lctx.resolver.borrow_mut().record_resolution(expr.id, def);
18171803
expr
18181804

18191805
}
@@ -1900,14 +1886,14 @@ fn pat_enum(lctx: &LoweringContext,
19001886
path: hir::Path,
19011887
subpats: hir::HirVec<P<hir::Pat>>)
19021888
-> P<hir::Pat> {
1903-
let def = lctx.resolve_generated_global_path(&path, true);
1889+
let def = lctx.resolver.borrow_mut().resolve_generated_global_path(&path, true);
19041890
let pt = if subpats.is_empty() {
19051891
hir::PatKind::Path(path)
19061892
} else {
19071893
hir::PatKind::TupleStruct(path, Some(subpats))
19081894
};
19091895
let pat = pat(lctx, span, pt);
1910-
lctx.record_def(pat.id, def);
1896+
lctx.resolver.borrow_mut().record_resolution(pat.id, def);
19111897
pat
19121898
}
19131899

@@ -1929,14 +1915,13 @@ fn pat_ident_binding_mode(lctx: &LoweringContext,
19291915

19301916
let pat = pat(lctx, span, pat_ident);
19311917

1932-
if let Some(ref resolver) = lctx.resolver {
1933-
let def_index =
1934-
resolver.borrow_mut().definitions()
1935-
.create_def_with_parent(lctx.parent_def.get(),
1936-
pat.id,
1937-
DefPathData::Binding(ident.name));
1938-
lctx.record_def(pat.id, Def::Local(DefId::local(def_index), pat.id));
1939-
}
1918+
let mut resolver = lctx.resolver.borrow_mut();
1919+
let def = resolver.definitions().map(|defs| {
1920+
let def_path_data = DefPathData::Binding(ident.name);
1921+
let def_index = defs.create_def_with_parent(lctx.parent_def.get(), pat.id, def_path_data);
1922+
Def::Local(DefId::local(def_index), pat.id)
1923+
}).unwrap_or(Def::Err);
1924+
resolver.record_resolution(pat.id, def);
19401925

19411926
pat
19421927
}

src/librustc_metadata/astencode.rs

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use rustc_serialize::{Encodable, EncoderHelpers};
5757
#[cfg(test)] use syntax::parse;
5858
#[cfg(test)] use syntax::ast::NodeId;
5959
#[cfg(test)] use rustc::hir::print as pprust;
60-
#[cfg(test)] use rustc::hir::lowering::{lower_item, LoweringContext};
60+
#[cfg(test)] use rustc::hir::lowering::{lower_item, LoweringContext, DummyResolver};
6161

6262
struct DecodeContext<'a, 'b, 'tcx: 'a> {
6363
tcx: &'a TyCtxt<'tcx>,
@@ -1325,6 +1325,14 @@ fn mk_ctxt() -> parse::ParseSess {
13251325
parse::ParseSess::new()
13261326
}
13271327

1328+
#[cfg(test)]
1329+
fn with_testing_context<T, F: FnOnce(LoweringContext) -> T>(f: F) -> T {
1330+
let assigner = FakeNodeIdAssigner;
1331+
let mut resolver = DummyResolver;
1332+
let lcx = LoweringContext::new(&assigner, None, &mut resolver);
1333+
f(lcx)
1334+
}
1335+
13281336
#[cfg(test)]
13291337
fn roundtrip(in_item: hir::Item) {
13301338
let mut wr = Cursor::new(Vec::new());
@@ -1338,34 +1346,34 @@ fn roundtrip(in_item: hir::Item) {
13381346
#[test]
13391347
fn test_basic() {
13401348
let cx = mk_ctxt();
1341-
let fnia = FakeNodeIdAssigner;
1342-
let lcx = LoweringContext::testing_context(&fnia);
1343-
roundtrip(lower_item(&lcx, &quote_item!(&cx,
1344-
fn foo() {}
1345-
).unwrap()));
1349+
with_testing_context(|lcx| {
1350+
roundtrip(lower_item(&lcx, &quote_item!(&cx,
1351+
fn foo() {}
1352+
).unwrap()));
1353+
});
13461354
}
13471355

13481356
#[test]
13491357
fn test_smalltalk() {
13501358
let cx = mk_ctxt();
1351-
let fnia = FakeNodeIdAssigner;
1352-
let lcx = LoweringContext::testing_context(&fnia);
1353-
roundtrip(lower_item(&lcx, &quote_item!(&cx,
1354-
fn foo() -> isize { 3 + 4 } // first smalltalk program ever executed.
1355-
).unwrap()));
1359+
with_testing_context(|lcx| {
1360+
roundtrip(lower_item(&lcx, &quote_item!(&cx,
1361+
fn foo() -> isize { 3 + 4 } // first smalltalk program ever executed.
1362+
).unwrap()));
1363+
});
13561364
}
13571365

13581366
#[test]
13591367
fn test_more() {
13601368
let cx = mk_ctxt();
1361-
let fnia = FakeNodeIdAssigner;
1362-
let lcx = LoweringContext::testing_context(&fnia);
1363-
roundtrip(lower_item(&lcx, &quote_item!(&cx,
1364-
fn foo(x: usize, y: usize) -> usize {
1365-
let z = x + y;
1366-
return z;
1367-
}
1368-
).unwrap()));
1369+
with_testing_context(|lcx| {
1370+
roundtrip(lower_item(&lcx, &quote_item!(&cx,
1371+
fn foo(x: usize, y: usize) -> usize {
1372+
let z = x + y;
1373+
return z;
1374+
}
1375+
).unwrap()));
1376+
});
13691377
}
13701378

13711379
#[test]
@@ -1377,21 +1385,22 @@ fn test_simplification() {
13771385
return alist {eq_fn: eq_int, data: Vec::new()};
13781386
}
13791387
).unwrap();
1380-
let fnia = FakeNodeIdAssigner;
1381-
let lcx = LoweringContext::testing_context(&fnia);
1382-
let hir_item = lower_item(&lcx, &item);
1383-
let item_in = InlinedItemRef::Item(&hir_item);
1384-
let item_out = simplify_ast(item_in);
1385-
let item_exp = InlinedItem::Item(P(lower_item(&lcx, &quote_item!(&cx,
1386-
fn new_int_alist<B>() -> alist<isize, B> {
1387-
return alist {eq_fn: eq_int, data: Vec::new()};
1388+
let cx = mk_ctxt();
1389+
with_testing_context(|lcx| {
1390+
let hir_item = lower_item(&lcx, &item);
1391+
let item_in = InlinedItemRef::Item(&hir_item);
1392+
let item_out = simplify_ast(item_in);
1393+
let item_exp = InlinedItem::Item(P(lower_item(&lcx, &quote_item!(&cx,
1394+
fn new_int_alist<B>() -> alist<isize, B> {
1395+
return alist {eq_fn: eq_int, data: Vec::new()};
1396+
}
1397+
).unwrap())));
1398+
match (item_out, item_exp) {
1399+
(InlinedItem::Item(item_out), InlinedItem::Item(item_exp)) => {
1400+
assert!(pprust::item_to_string(&item_out) ==
1401+
pprust::item_to_string(&item_exp));
1402+
}
1403+
_ => bug!()
13881404
}
1389-
).unwrap())));
1390-
match (item_out, item_exp) {
1391-
(InlinedItem::Item(item_out), InlinedItem::Item(item_exp)) => {
1392-
assert!(pprust::item_to_string(&item_out) ==
1393-
pprust::item_to_string(&item_exp));
1394-
}
1395-
_ => bug!()
1396-
}
1405+
});
13971406
}

src/librustc_resolve/lib.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,11 +1098,12 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
10981098
}
10991099
}
11001100

1101-
fn def_map(&mut self) -> &mut DefMap {
1102-
&mut self.def_map
1101+
fn record_resolution(&mut self, id: NodeId, def: Def) {
1102+
self.def_map.insert(id, PathResolution { base_def: def, depth: 0 });
11031103
}
1104-
fn definitions(&mut self) -> &mut Definitions {
1105-
self.definitions
1104+
1105+
fn definitions(&mut self) -> Option<&mut Definitions> {
1106+
Some(self.definitions)
11061107
}
11071108
}
11081109

0 commit comments

Comments
 (0)