Skip to content

Commit 63ac2aa

Browse files
committed
Fix tests and assertions; add some comments
1 parent 59cb170 commit 63ac2aa

26 files changed

+274
-187
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
}
@@ -471,8 +474,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) {
471474
}
472475
}
473476
ItemKind::Use(ref path, _) => {
474-
visitor.visit_id(item.id);
475-
visitor.visit_path(path, item.hir_id);
477+
visitor.visit_use(path, item.id, item.hir_id);
476478
}
477479
ItemKind::Static(ref typ, _, body) |
478480
ItemKind::Const(ref typ, body) => {
@@ -554,6 +556,14 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) {
554556
walk_list!(visitor, visit_attribute, &item.attrs);
555557
}
556558

559+
pub fn walk_use<'v, V: Visitor<'v>>(visitor: &mut V,
560+
path: &'v Path,
561+
item_id: NodeId,
562+
hir_id: HirId) {
563+
visitor.visit_id(item_id);
564+
visitor.visit_path(path, hir_id);
565+
}
566+
557567
pub fn walk_enum_def<'v, V: Visitor<'v>>(visitor: &mut V,
558568
enum_definition: &'v EnumDef,
559569
generics: &'v Generics,
@@ -652,6 +662,9 @@ pub fn walk_path_segment<'v, V: Visitor<'v>>(visitor: &mut V,
652662
path_span: Span,
653663
segment: &'v PathSegment) {
654664
visitor.visit_ident(segment.ident);
665+
if let Some(id) = segment.id {
666+
visitor.visit_id(id);
667+
}
655668
if let Some(ref args) = segment.args {
656669
visitor.visit_generic_args(path_span, args);
657670
}

src/librustc/hir/lowering.rs

+79-31
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,9 @@ impl<'a> LoweringContext<'a> {
10691069
}
10701070

10711071
fn lower_attr(&mut self, attr: &Attribute) -> Attribute {
1072+
// Note that we explicitly do not walk the path. Since we don't really
1073+
// lower attributes (we use the AST version) there is nowhere to keep
1074+
// the HirIds. We don't actually need HIR version of attributes anyway.
10721075
Attribute {
10731076
id: attr.id,
10741077
style: attr.style,
@@ -1682,6 +1685,7 @@ impl<'a> LoweringContext<'a> {
16821685
num_lifetimes,
16831686
parenthesized_generic_args,
16841687
itctx.reborrow(),
1688+
None,
16851689
)
16861690
})
16871691
.collect(),
@@ -1725,6 +1729,7 @@ impl<'a> LoweringContext<'a> {
17251729
0,
17261730
ParenthesizedGenericArgs::Warn,
17271731
itctx.reborrow(),
1732+
None,
17281733
));
17291734
let qpath = hir::QPath::TypeRelative(ty, segment);
17301735

@@ -1753,6 +1758,7 @@ impl<'a> LoweringContext<'a> {
17531758
p: &Path,
17541759
ident: Option<Ident>,
17551760
param_mode: ParamMode,
1761+
explicit_owner: Option<NodeId>,
17561762
) -> hir::Path {
17571763
hir::Path {
17581764
def,
@@ -1766,6 +1772,7 @@ impl<'a> LoweringContext<'a> {
17661772
0,
17671773
ParenthesizedGenericArgs::Err,
17681774
ImplTraitContext::disallowed(),
1775+
explicit_owner,
17691776
)
17701777
})
17711778
.chain(ident.map(|ident| hir::PathSegment::from_ident(ident)))
@@ -1776,7 +1783,7 @@ impl<'a> LoweringContext<'a> {
17761783

17771784
fn lower_path(&mut self, id: NodeId, p: &Path, param_mode: ParamMode) -> hir::Path {
17781785
let def = self.expect_full_def(id);
1779-
self.lower_path_extra(def, p, None, param_mode)
1786+
self.lower_path_extra(def, p, None, param_mode, None)
17801787
}
17811788

17821789
fn lower_path_segment(
@@ -1787,6 +1794,7 @@ impl<'a> LoweringContext<'a> {
17871794
expected_lifetimes: usize,
17881795
parenthesized_generic_args: ParenthesizedGenericArgs,
17891796
itctx: ImplTraitContext<'_>,
1797+
explicit_owner: Option<NodeId>,
17901798
) -> hir::PathSegment {
17911799
let (mut generic_args, infer_types) = if let Some(ref generic_args) = segment.args {
17921800
let msg = "parenthesized parameters may only be used with a trait";
@@ -1858,9 +1866,15 @@ impl<'a> LoweringContext<'a> {
18581866
}
18591867

18601868
let def = self.expect_full_def(segment.id);
1869+
let id = if let Some(owner) = explicit_owner {
1870+
self.lower_node_id_with_owner(segment.id, owner)
1871+
} else {
1872+
self.lower_node_id(segment.id)
1873+
};
1874+
18611875
hir::PathSegment::new(
18621876
segment.ident,
1863-
Some(segment.id),
1877+
Some(id.node_id),
18641878
Some(def),
18651879
generic_args,
18661880
infer_types,
@@ -2944,19 +2958,20 @@ impl<'a> LoweringContext<'a> {
29442958
attrs: &hir::HirVec<Attribute>,
29452959
) -> hir::ItemKind {
29462960
let path = &tree.prefix;
2961+
let segments = prefix
2962+
.segments
2963+
.iter()
2964+
.chain(path.segments.iter())
2965+
.cloned()
2966+
.collect();
29472967

29482968
match tree.kind {
29492969
UseTreeKind::Simple(rename, id1, id2) => {
29502970
*name = tree.ident().name;
29512971

29522972
// First apply the prefix to the path
29532973
let mut path = Path {
2954-
segments: prefix
2955-
.segments
2956-
.iter()
2957-
.chain(path.segments.iter())
2958-
.cloned()
2959-
.collect(),
2974+
segments,
29602975
span: path.span,
29612976
};
29622977

@@ -2976,9 +2991,18 @@ impl<'a> LoweringContext<'a> {
29762991
// for later
29772992
let ret_def = defs.next().unwrap_or(Def::Err);
29782993

2994+
// Here, we are looping over namespaces, if they exist for the definition
2995+
// being imported. We only handle type and value namespaces because we
2996+
// won't be dealing with macros in the rest of the compiler.
2997+
// Essentially a single `use` which imports two names is desugared into
2998+
// two imports.
29792999
for (def, &new_node_id) in defs.zip([id1, id2].iter()) {
29803000
let vis = vis.clone();
29813001
let name = name.clone();
3002+
let mut path = path.clone();
3003+
for seg in &mut path.segments {
3004+
seg.id = self.sess.next_node_id();
3005+
}
29823006
let span = path.span;
29833007
self.resolver.definitions().create_def_with_parent(
29843008
parent_def_index,
@@ -2991,7 +3015,8 @@ impl<'a> LoweringContext<'a> {
29913015

29923016
self.with_hir_id_owner(new_node_id, |this| {
29933017
let new_id = this.lower_node_id(new_node_id);
2994-
let path = this.lower_path_extra(def, &path, None, ParamMode::Explicit);
3018+
let path =
3019+
this.lower_path_extra(def, &path, None, ParamMode::Explicit, None);
29953020
let item = hir::ItemKind::Use(P(path), hir::UseKind::Single);
29963021
let vis_kind = match vis.node {
29973022
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
@@ -3001,7 +3026,6 @@ impl<'a> LoweringContext<'a> {
30013026
let id = this.next_id();
30023027
hir::VisibilityKind::Restricted {
30033028
path: path.clone(),
3004-
// We are allocating a new NodeId here
30053029
id: id.node_id,
30063030
hir_id: id.hir_id,
30073031
}
@@ -3024,50 +3048,60 @@ impl<'a> LoweringContext<'a> {
30243048
});
30253049
}
30263050

3027-
let path = P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit));
3051+
let path =
3052+
P(self.lower_path_extra(ret_def, &path, None, ParamMode::Explicit, None));
30283053
hir::ItemKind::Use(path, hir::UseKind::Single)
30293054
}
30303055
UseTreeKind::Glob => {
30313056
let path = P(self.lower_path(
30323057
id,
30333058
&Path {
3034-
segments: prefix
3035-
.segments
3036-
.iter()
3037-
.chain(path.segments.iter())
3038-
.cloned()
3039-
.collect(),
3059+
segments,
30403060
span: path.span,
30413061
},
30423062
ParamMode::Explicit,
30433063
));
30443064
hir::ItemKind::Use(path, hir::UseKind::Glob)
30453065
}
30463066
UseTreeKind::Nested(ref trees) => {
3067+
// Nested imports are desugared into simple imports.
3068+
30473069
let prefix = Path {
3048-
segments: prefix
3049-
.segments
3050-
.iter()
3051-
.chain(path.segments.iter())
3052-
.cloned()
3053-
.collect(),
3070+
segments,
30543071
span: prefix.span.to(path.span),
30553072
};
30563073

3057-
// Add all the nested PathListItems in the HIR
3074+
// Add all the nested PathListItems to the HIR.
30583075
for &(ref use_tree, id) in trees {
30593076
self.allocate_hir_id_counter(id, &use_tree);
3077+
30603078
let LoweredNodeId {
30613079
node_id: new_id,
30623080
hir_id: new_hir_id,
30633081
} = self.lower_node_id(id);
30643082

30653083
let mut vis = vis.clone();
30663084
let mut name = name.clone();
3067-
let item =
3068-
self.lower_use_tree(use_tree, &prefix, new_id, &mut vis, &mut name, &attrs);
3085+
let mut prefix = prefix.clone();
30693086

3087+
// Give the segments new ids since they are being cloned.
3088+
for seg in &mut prefix.segments {
3089+
seg.id = self.sess.next_node_id();
3090+
}
3091+
3092+
// Each `use` import is an item and thus are owners of the
3093+
// names in the path. Up to this point the nested import is
3094+
// the current owner, since we want each desugared import to
3095+
// own its own names, we have to adjust the owner before
3096+
// lowering the rest of the import.
30703097
self.with_hir_id_owner(new_id, |this| {
3098+
let item = this.lower_use_tree(use_tree,
3099+
&prefix,
3100+
new_id,
3101+
&mut vis,
3102+
&mut name,
3103+
attrs);
3104+
30713105
let vis_kind = match vis.node {
30723106
hir::VisibilityKind::Public => hir::VisibilityKind::Public,
30733107
hir::VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar),
@@ -3076,7 +3110,6 @@ impl<'a> LoweringContext<'a> {
30763110
let id = this.next_id();
30773111
hir::VisibilityKind::Restricted {
30783112
path: path.clone(),
3079-
// We are allocating a new NodeId here
30803113
id: id.node_id,
30813114
hir_id: id.hir_id,
30823115
}
@@ -3089,7 +3122,7 @@ impl<'a> LoweringContext<'a> {
30893122
hir::Item {
30903123
id: new_id,
30913124
hir_id: new_hir_id,
3092-
name: name,
3125+
name,
30933126
attrs: attrs.clone(),
30943127
node: item,
30953128
vis,
@@ -3653,6 +3686,7 @@ impl<'a> LoweringContext<'a> {
36533686
0,
36543687
ParenthesizedGenericArgs::Err,
36553688
ImplTraitContext::disallowed(),
3689+
None,
36563690
);
36573691
let args = args.iter().map(|x| self.lower_expr(x)).collect();
36583692
hir::ExprKind::MethodCall(hir_seg, seg.ident.span, args)
@@ -4506,8 +4540,15 @@ impl<'a> LoweringContext<'a> {
45064540
} else {
45074541
self.lower_node_id(id)
45084542
};
4543+
let def = self.expect_full_def(id);
45094544
hir::VisibilityKind::Restricted {
4510-
path: P(self.lower_path(id, path, ParamMode::Explicit)),
4545+
path: P(self.lower_path_extra(
4546+
def,
4547+
path,
4548+
None,
4549+
ParamMode::Explicit,
4550+
explicit_owner,
4551+
)),
45114552
id: lowered_id.node_id,
45124553
hir_id: lowered_id.hir_id,
45134554
}
@@ -4814,8 +4855,15 @@ impl<'a> LoweringContext<'a> {
48144855
params: Option<P<hir::GenericArgs>>,
48154856
is_value: bool
48164857
) -> hir::Path {
4817-
self.resolver
4818-
.resolve_str_path(span, self.crate_root, components, params, is_value)
4858+
let mut path = self.resolver
4859+
.resolve_str_path(span, self.crate_root, components, params, is_value);
4860+
4861+
for seg in path.segments.iter_mut() {
4862+
if let Some(id) = seg.id {
4863+
seg.id = Some(self.lower_node_id(id).node_id);
4864+
}
4865+
}
4866+
path
48194867
}
48204868

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

src/librustc/hir/map/collector.rs

+10-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

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 {

src/librustc/hir/map/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,7 @@ impl<'a> print::State<'a> {
11001100
Node::AnonConst(a) => self.print_anon_const(&a),
11011101
Node::Expr(a) => self.print_expr(&a),
11021102
Node::Stmt(a) => self.print_stmt(&a),
1103-
Node::PathSegment(_) => bug!("cannot print PathSegment"),
1103+
Node::PathSegment(a) => self.print_path_segment(&a),
11041104
Node::Ty(a) => self.print_type(&a),
11051105
Node::TraitRef(a) => self.print_trait_ref(&a),
11061106
Node::Binding(a) |

0 commit comments

Comments
 (0)