Skip to content

Commit 81ee74b

Browse files
nrcnrc
committed
Fix tests and assertions; add some comments
1 parent 5a32da5 commit 81ee74b

File tree

7 files changed

+135
-43
lines changed

7 files changed

+135
-43
lines changed

‎src/librustc/hir/intravisit.rs

+15-2
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,9 @@ pub trait Visitor<'v> : Sized {
298298
fn visit_fn(&mut self, fk: FnKind<'v>, fd: &'v FnDecl, b: BodyId, s: Span, id: NodeId) {
299299
walk_fn(self, fk, fd, b, s, id)
300300
}
301+
fn visit_use(&mut self, path: &'v Path, id: NodeId, hir_id: HirId) {
302+
walk_use(self, path, id, hir_id)
303+
}
301304
fn visit_trait_item(&mut self, ti: &'v TraitItem) {
302305
walk_trait_item(self, ti)
303306
}
@@ -469,8 +472,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) {
469472
}
470473
}
471474
ItemKind::Use(ref path, _) => {
472-
visitor.visit_id(item.id);
473-
visitor.visit_path(path, item.hir_id);
475+
visitor.visit_use(path, item.id, item.hir_id);
474476
}
475477
ItemKind::Static(ref typ, _, body) |
476478
ItemKind::Const(ref typ, body) => {
@@ -552,6 +554,14 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) {
552554
walk_list!(visitor, visit_attribute, &item.attrs);
553555
}
554556

557+
pub fn walk_use<'v, V: Visitor<'v>>(visitor: &mut V,
558+
path: &'v Path,
559+
item_id: NodeId,
560+
hir_id: HirId) {
561+
visitor.visit_id(item_id);
562+
visitor.visit_path(path, hir_id);
563+
}
564+
555565
pub fn walk_enum_def<'v, V: Visitor<'v>>(visitor: &mut V,
556566
enum_definition: &'v EnumDef,
557567
generics: &'v Generics,
@@ -650,6 +660,9 @@ pub fn walk_path_segment<'v, V: Visitor<'v>>(visitor: &mut V,
650660
path_span: Span,
651661
segment: &'v PathSegment) {
652662
visitor.visit_ident(segment.ident);
663+
if let Some(id) = segment.id {
664+
visitor.visit_id(id);
665+
}
653666
if let Some(ref args) = segment.args {
654667
visitor.visit_generic_args(path_span, args);
655668
}

‎src/librustc/hir/lowering.rs

+79-31
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,9 @@ impl<'a> LoweringContext<'a> {
10651065
}
10661066

10671067
fn lower_attr(&mut self, attr: &Attribute) -> Attribute {
1068+
// Note that we explicitly do not walk the path. Since we don't really
1069+
// lower attributes (we use the AST version) there is nowhere to keep
1070+
// the HirIds. We don't actually need HIR version of attributes anyway.
10681071
Attribute {
10691072
id: attr.id,
10701073
style: attr.style,
@@ -1678,6 +1681,7 @@ impl<'a> LoweringContext<'a> {
16781681
num_lifetimes,
16791682
parenthesized_generic_args,
16801683
itctx.reborrow(),
1684+
None,
16811685
)
16821686
})
16831687
.collect(),
@@ -1721,6 +1725,7 @@ impl<'a> LoweringContext<'a> {
17211725
0,
17221726
ParenthesizedGenericArgs::Warn,
17231727
itctx.reborrow(),
1728+
None,
17241729
));
17251730
let qpath = hir::QPath::TypeRelative(ty, segment);
17261731

@@ -1749,6 +1754,7 @@ impl<'a> LoweringContext<'a> {
17491754
p: &Path,
17501755
ident: Option<Ident>,
17511756
param_mode: ParamMode,
1757+
explicit_owner: Option<NodeId>,
17521758
) -> hir::Path {
17531759
hir::Path {
17541760
def,
@@ -1762,6 +1768,7 @@ impl<'a> LoweringContext<'a> {
17621768
0,
17631769
ParenthesizedGenericArgs::Err,
17641770
ImplTraitContext::disallowed(),
1771+
explicit_owner,
17651772
)
17661773
})
17671774
.chain(ident.map(|ident| hir::PathSegment::from_ident(ident)))
@@ -1772,7 +1779,7 @@ impl<'a> LoweringContext<'a> {
17721779

17731780
fn lower_path(&mut self, id: NodeId, p: &Path, param_mode: ParamMode) -> hir::Path {
17741781
let def = self.expect_full_def(id);
1775-
self.lower_path_extra(def, p, None, param_mode)
1782+
self.lower_path_extra(def, p, None, param_mode, None)
17761783
}
17771784

17781785
fn lower_path_segment(
@@ -1783,6 +1790,7 @@ impl<'a> LoweringContext<'a> {
17831790
expected_lifetimes: usize,
17841791
parenthesized_generic_args: ParenthesizedGenericArgs,
17851792
itctx: ImplTraitContext<'_>,
1793+
explicit_owner: Option<NodeId>,
17861794
) -> hir::PathSegment {
17871795
let (mut generic_args, infer_types) = if let Some(ref generic_args) = segment.args {
17881796
let msg = "parenthesized parameters may only be used with a trait";
@@ -1854,9 +1862,15 @@ impl<'a> LoweringContext<'a> {
18541862
}
18551863

18561864
let def = self.expect_full_def(segment.id);
1865+
let id = if let Some(owner) = explicit_owner {
1866+
self.lower_node_id_with_owner(segment.id, owner)
1867+
} else {
1868+
self.lower_node_id(segment.id)
1869+
};
1870+
18571871
hir::PathSegment::new(
18581872
segment.ident,
1859-
Some(segment.id),
1873+
Some(id.node_id),
18601874
Some(def),
18611875
generic_args,
18621876
infer_types,
@@ -2921,19 +2935,20 @@ impl<'a> LoweringContext<'a> {
29212935
attrs: &hir::HirVec<Attribute>,
29222936
) -> hir::ItemKind {
29232937
let path = &tree.prefix;
2938+
let segments = prefix
2939+
.segments
2940+
.iter()
2941+
.chain(path.segments.iter())
2942+
.cloned()
2943+
.collect();
29242944

29252945
match tree.kind {
29262946
UseTreeKind::Simple(rename, id1, id2) => {
29272947
*name = tree.ident().name;
29282948

29292949
// First apply the prefix to the path
29302950
let mut path = Path {
2931-
segments: prefix
2932-
.segments
2933-
.iter()
2934-
.chain(path.segments.iter())
2935-
.cloned()
2936-
.collect(),
2951+
segments,
29372952
span: path.span,
29382953
};
29392954

@@ -2953,9 +2968,18 @@ impl<'a> LoweringContext<'a> {
29532968
// for later
29542969
let ret_def = defs.next().unwrap_or(Def::Err);
29552970

2971+
// Here, we are looping over namespaces, if they exist for the definition
2972+
// being imported. We only handle type and value namespaces because we
2973+
// won't be dealing with macros in the rest of the compiler.
2974+
// Essentially a single `use` which imports two names is desugared into
2975+
// two imports.
29562976
for (def, &new_node_id) in defs.zip([id1, id2].iter()) {
29572977
let vis = vis.clone();
29582978
let name = name.clone();
2979+
let mut path = path.clone();
2980+
for seg in &mut path.segments {
2981+
seg.id = self.sess.next_node_id();
2982+
}
29592983
let span = path.span;
29602984
self.resolver.definitions().create_def_with_parent(
29612985
parent_def_index,
@@ -2968,7 +2992,8 @@ impl<'a> LoweringContext<'a> {
29682992

29692993
self.with_hir_id_owner(new_node_id, |this| {
29702994
let new_id = this.lower_node_id(new_node_id);
2971-
let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
2995+
let path =
2996+
this.lower_path_extra(def, &path, None, ParamMode::Explicit, None);
29722997
let item = hir::ItemKind::Use(P(path), hir::UseKind::Single);
29732998
let vis_kind = match vis.node {
29742999
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
@@ -2978,7 +3003,6 @@ impl<'a> LoweringContext<'a> {
29783003
let id = this.next_id();
29793004
hir::VisibilityKind::Restricted {
29803005
path: path.clone(),
2981-
// We are allocating a new NodeId here
29823006
id: id.node_id,
29833007
hir_id: id.hir_id,
29843008
}
@@ -3001,50 +3025,60 @@ impl<'a> LoweringContext<'a> {
30013025
});
30023026
}
30033027

3004-
let path = P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit));
3028+
let path =
3029+
P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit, None));
30053030
hir::ItemKind::Use(path, hir::UseKind::Single)
30063031
}
30073032
UseTreeKind::Glob => {
30083033
let path = P(self.lower_path(
30093034
id,
30103035
&Path {
3011-
segments: prefix
3012-
.segments
3013-
.iter()
3014-
.chain(path.segments.iter())
3015-
.cloned()
3016-
.collect(),
3036+
segments,
30173037
span: path.span,
30183038
},
30193039
ParamMode::Explicit,
30203040
));
30213041
hir::ItemKind::Use(path, hir::UseKind::Glob)
30223042
}
30233043
UseTreeKind::Nested(ref trees) => {
3044+
// Nested imports are desugared into simple imports.
3045+
30243046
let prefix = Path {
3025-
segments: prefix
3026-
.segments
3027-
.iter()
3028-
.chain(path.segments.iter())
3029-
.cloned()
3030-
.collect(),
3047+
segments,
30313048
span: prefix.span.to(path.span),
30323049
};
30333050

3034-
// Add all the nested PathListItems in the HIR
3051+
// Add all the nested PathListItems to the HIR.
30353052
for &(ref use_tree, id) in trees {
30363053
self.allocate_hir_id_counter(id, &use_tree);
3054+
30373055
let LoweredNodeId {
30383056
node_id: new_id,
30393057
hir_id: new_hir_id,
30403058
} = self.lower_node_id(id);
30413059

30423060
let mut vis = vis.clone();
30433061
let mut name = name.clone();
3044-
let item =
3045-
self.lower_use_tree(use_tree, &prefix, new_id, &mut vis, &mut name, &attrs);
3062+
let mut prefix = prefix.clone();
30463063

3064+
// Give the segments new ids since they are being cloned.
3065+
for seg in &mut prefix.segments {
3066+
seg.id = self.sess.next_node_id();
3067+
}
3068+
3069+
// Each `use` import is an item and thus are owners of the
3070+
// names in the path. Up to this point the nested import is
3071+
// the current owner, since we want each desugared import to
3072+
// own its own names, we have to adjust the owner before
3073+
// lowering the rest of the import.
30473074
self.with_hir_id_owner(new_id, |this| {
3075+
let item = this.lower_use_tree(use_tree,
3076+
&prefix,
3077+
new_id,
3078+
&mut vis,
3079+
&mut name,
3080+
attrs);
3081+
30483082
let vis_kind = match vis.node {
30493083
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
30503084
hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
@@ -3053,7 +3087,6 @@ impl<'a> LoweringContext<'a> {
30533087
let id = this.next_id();
30543088
hir::VisibilityKind::Restricted {
30553089
path: path.clone(),
3056-
// We are allocating a new NodeId here
30573090
id: id.node_id,
30583091
hir_id: id.hir_id,
30593092
}
@@ -3066,7 +3099,7 @@ impl<'a> LoweringContext<'a> {
30663099
hir::Item {
30673100
id: new_id,
30683101
hir_id: new_hir_id,
3069-
name: name,
3102+
name,
30703103
attrs: attrs.clone(),
30713104
node: item,
30723105
vis,
@@ -3630,6 +3663,7 @@ impl<'a> LoweringContext<'a> {
36303663
0,
36313664
ParenthesizedGenericArgs::Err,
36323665
ImplTraitContext::disallowed(),
3666+
None,
36333667
);
36343668
let args = args.iter().map(|x| self.lower_expr(x)).collect();
36353669
hir::ExprKind::MethodCall(hir_seg, seg.ident.span, args)
@@ -4483,8 +4517,15 @@ impl<'a> LoweringContext<'a> {
44834517
} else {
44844518
self.lower_node_id(id)
44854519
};
4520+
let def = self.expect_full_def(id);
44864521
hir::VisibilityKind::Restricted {
4487-
path: P(self.lower_path(id, path, ParamMode::Explicit)),
4522+
path: P(self.lower_path_extra(
4523+
def,
4524+
path,
4525+
None,
4526+
ParamMode::Explicit,
4527+
explicit_owner,
4528+
)),
44884529
id: lowered_id.node_id,
44894530
hir_id: lowered_id.hir_id,
44904531
}
@@ -4791,8 +4832,15 @@ impl<'a> LoweringContext<'a> {
47914832
params: Option<P<hir::GenericArgs>>,
47924833
is_value: bool
47934834
) -> hir::Path {
4794-
self.resolver
4795-
.resolve_str_path(span, self.crate_root, components, params, is_value)
4835+
let mut path = self.resolver
4836+
.resolve_str_path(span, self.crate_root, components, params, is_value);
4837+
4838+
for seg in path.segments.iter_mut() {
4839+
if let Some(id) = seg.id {
4840+
seg.id = Some(self.lower_node_id(id).node_id);
4841+
}
4842+
}
4843+
path
47964844
}
47974845

47984846
fn ty_path(&mut self, id: LoweredNodeId, span: Span, qpath: hir::QPath) -> hir::Ty {

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

+16-5
Original file line numberDiff line numberDiff line change
@@ -210,17 +210,22 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
210210
None => format!("{:?}", node)
211211
};
212212

213-
if hir_id == ::hir::DUMMY_HIR_ID {
214-
debug!("Maybe you forgot to lower the node id {:?}?", id);
215-
}
213+
let forgot_str = if hir_id == ::hir::DUMMY_HIR_ID {
214+
format!("\nMaybe you forgot to lower the node id {:?}?", id)
215+
} else {
216+
String::new()
217+
};
216218

217219
bug!("inconsistent DepNode for `{}`: \
218-
current_dep_node_owner={}, hir_id.owner={}",
220+
current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?}) {}",
219221
node_str,
220222
self.definitions
221223
.def_path(self.current_dep_node_owner)
222224
.to_string_no_crate(),
223-
self.definitions.def_path(hir_id.owner).to_string_no_crate())
225+
self.current_dep_node_owner,
226+
self.definitions.def_path(hir_id.owner).to_string_no_crate(),
227+
hir_id.owner,
228+
forgot_str)
224229
}
225230
}
226231

@@ -317,6 +322,12 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
317322
});
318323
}
319324

325+
fn visit_use(&mut self, path: &'hir Path, id: NodeId, hir_id: HirId) {
326+
self.with_dep_node_owner(hir_id.owner, path, |this| {
327+
intravisit::walk_use(this, path, id, hir_id)
328+
});
329+
}
330+
320331
fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem) {
321332
self.insert(foreign_item.id, Node::ForeignItem(foreign_item));
322333

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

+8-4
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> {
8888
walk(self);
8989

9090
if owner_def_index == CRATE_DEF_INDEX {
91-
return
91+
return;
9292
}
9393

9494
// There's always at least one entry for the owning item itself
@@ -129,13 +129,16 @@ impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> {
129129
local_id,
130130
self.hir_map.node_to_string(node_id)));
131131
}
132-
133132
self.errors.push(format!(
134133
"ItemLocalIds not assigned densely in {}. \
135-
Max ItemLocalId = {}, missing IDs = {:?}",
134+
Max ItemLocalId = {}, missing IDs = {:?}; seens IDs = {:?}",
136135
self.hir_map.def_path(DefId::local(owner_def_index)).to_string_no_crate(),
137136
max,
138-
missing_items));
137+
missing_items,
138+
self.hir_ids_seen
139+
.values()
140+
.map(|n| format!("({:?} {})", n, self.hir_map.node_to_string(*n)))
141+
.collect::<Vec<_>>()));
139142
}
140143
}
141144
}
@@ -155,6 +158,7 @@ impl<'a, 'hir: 'a> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> {
155158
self.errors.push(format!("HirIdValidator: No HirId assigned for NodeId {}: {:?}",
156159
node_id,
157160
self.hir_map.node_to_string(node_id)));
161+
return;
158162
}
159163

160164
if owner != stable_id.owner {

0 commit comments

Comments
 (0)