From d0a174d41b230dc2cb08a902c2ecca0d89ee7856 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 13:35:14 -0500 Subject: [PATCH 01/10] track the span for each id so that we can give a nice ICE --- src/librustc/hir/map/collector.rs | 64 ++++++++++++++++++------------- src/librustc/hir/map/mod.rs | 4 +- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index 4fbcd83adb555..2917fd7457acf 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -28,6 +28,10 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHashe pub(super) struct NodeCollector<'a, 'hir> { /// The crate krate: &'hir Crate, + + /// Source map + source_map: &'a SourceMap, + /// The node map map: Vec>>, /// The parent of this node @@ -54,7 +58,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { pub(super) fn root(krate: &'hir Crate, dep_graph: &'a DepGraph, definitions: &'a definitions::Definitions, - hcx: StableHashingContext<'a>) + hcx: StableHashingContext<'a>, + source_map: &'a SourceMap) -> NodeCollector<'a, 'hir> { let root_mod_def_path_hash = definitions.def_path_hash(CRATE_DEF_INDEX); @@ -102,6 +107,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { let mut collector = NodeCollector { krate, + source_map, map: vec![], parent_node: CRATE_NODE_ID, current_signature_dep_index: root_mod_sig_dep_index, @@ -125,7 +131,6 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { pub(super) fn finalize_and_compute_crate_hash(mut self, crate_disambiguator: CrateDisambiguator, cstore: &dyn CrateStore, - source_map: &SourceMap, commandline_args_hash: u64) -> (Vec>>, Svh) { @@ -154,7 +159,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { // If we included the full mapping in the SVH, we could only have // reproducible builds by compiling from the same directory. So we just // hash the result of the mapping instead of the mapping itself. - let mut source_file_names: Vec<_> = source_map + let mut source_file_names: Vec<_> = self + .source_map .files() .iter() .filter(|source_file| CrateNum::from_u32(source_file.crate_of_origin) == LOCAL_CRATE) @@ -186,7 +192,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { self.map[id.as_usize()] = Some(entry); } - fn insert(&mut self, id: NodeId, node: Node<'hir>) { + fn insert(&mut self, span: Span, id: NodeId, node: Node<'hir>) { let entry = Entry { parent: self.parent_node, dep_node: if self.currently_in_body { @@ -216,8 +222,11 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { String::new() }; - bug!("inconsistent DepNode for `{}`: \ - current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?}){}", + span_bug!( + span, + "inconsistent DepNode at `{:?}` for `{}`: \ + current_dep_node_owner={} ({:?}), hir_id.owner={} ({:?}){}", + self.source_map.span_to_string(span), node_str, self.definitions .def_path(self.current_dep_node_owner) @@ -225,7 +234,8 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { self.current_dep_node_owner, self.definitions.def_path(hir_id.owner).to_string_no_crate(), hir_id.owner, - forgot_str) + forgot_str, + ) } } @@ -309,12 +319,12 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { debug_assert_eq!(i.hir_id.owner, self.definitions.opt_def_index(i.id).unwrap()); self.with_dep_node_owner(i.hir_id.owner, i, |this| { - this.insert(i.id, Node::Item(i)); + this.insert(i.span, i.id, Node::Item(i)); this.with_parent(i.id, |this| { if let ItemKind::Struct(ref struct_def, _) = i.node { // If this is a tuple-like struct, register the constructor. if !struct_def.is_struct() { - this.insert(struct_def.id(), Node::StructCtor(struct_def)); + this.insert(i.span, struct_def.id(), Node::StructCtor(struct_def)); } } intravisit::walk_item(this, i); @@ -323,7 +333,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem) { - self.insert(foreign_item.id, Node::ForeignItem(foreign_item)); + self.insert(foreign_item.span, foreign_item.id, Node::ForeignItem(foreign_item)); self.with_parent(foreign_item.id, |this| { intravisit::walk_foreign_item(this, foreign_item); @@ -331,7 +341,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_generic_param(&mut self, param: &'hir GenericParam) { - self.insert(param.id, Node::GenericParam(param)); + self.insert(param.span, param.id, Node::GenericParam(param)); intravisit::walk_generic_param(self, param); } @@ -339,7 +349,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { debug_assert_eq!(ti.hir_id.owner, self.definitions.opt_def_index(ti.id).unwrap()); self.with_dep_node_owner(ti.hir_id.owner, ti, |this| { - this.insert(ti.id, Node::TraitItem(ti)); + this.insert(ti.span, ti.id, Node::TraitItem(ti)); this.with_parent(ti.id, |this| { intravisit::walk_trait_item(this, ti); @@ -351,7 +361,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { debug_assert_eq!(ii.hir_id.owner, self.definitions.opt_def_index(ii.id).unwrap()); self.with_dep_node_owner(ii.hir_id.owner, ii, |this| { - this.insert(ii.id, Node::ImplItem(ii)); + this.insert(ii.span, ii.id, Node::ImplItem(ii)); this.with_parent(ii.id, |this| { intravisit::walk_impl_item(this, ii); @@ -365,7 +375,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } else { Node::Pat(pat) }; - self.insert(pat.id, node); + self.insert(pat.span, pat.id, node); self.with_parent(pat.id, |this| { intravisit::walk_pat(this, pat); @@ -373,7 +383,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_anon_const(&mut self, constant: &'hir AnonConst) { - self.insert(constant.id, Node::AnonConst(constant)); + self.insert(DUMMY_SP, constant.id, Node::AnonConst(constant)); self.with_parent(constant.id, |this| { intravisit::walk_anon_const(this, constant); @@ -381,7 +391,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_expr(&mut self, expr: &'hir Expr) { - self.insert(expr.id, Node::Expr(expr)); + self.insert(expr.span, expr.id, Node::Expr(expr)); self.with_parent(expr.id, |this| { intravisit::walk_expr(this, expr); @@ -390,7 +400,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { fn visit_stmt(&mut self, stmt: &'hir Stmt) { let id = stmt.node.id(); - self.insert(id, Node::Stmt(stmt)); + self.insert(stmt.span, id, Node::Stmt(stmt)); self.with_parent(id, |this| { intravisit::walk_stmt(this, stmt); @@ -399,13 +409,13 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { fn visit_path_segment(&mut self, path_span: Span, path_segment: &'hir PathSegment) { if let Some(id) = path_segment.id { - self.insert(id, Node::PathSegment(path_segment)); + self.insert(path_span, id, Node::PathSegment(path_segment)); } intravisit::walk_path_segment(self, path_span, path_segment); } fn visit_ty(&mut self, ty: &'hir Ty) { - self.insert(ty.id, Node::Ty(ty)); + self.insert(ty.span, ty.id, Node::Ty(ty)); self.with_parent(ty.id, |this| { intravisit::walk_ty(this, ty); @@ -413,7 +423,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_trait_ref(&mut self, tr: &'hir TraitRef) { - self.insert(tr.ref_id, Node::TraitRef(tr)); + self.insert(tr.path.span, tr.ref_id, Node::TraitRef(tr)); self.with_parent(tr.ref_id, |this| { intravisit::walk_trait_ref(this, tr); @@ -427,21 +437,21 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_block(&mut self, block: &'hir Block) { - self.insert(block.id, Node::Block(block)); + self.insert(block.span, block.id, Node::Block(block)); self.with_parent(block.id, |this| { intravisit::walk_block(this, block); }); } fn visit_local(&mut self, l: &'hir Local) { - self.insert(l.id, Node::Local(l)); + self.insert(l.span, l.id, Node::Local(l)); self.with_parent(l.id, |this| { intravisit::walk_local(this, l) }) } fn visit_lifetime(&mut self, lifetime: &'hir Lifetime) { - self.insert(lifetime.id, Node::Lifetime(lifetime)); + self.insert(lifetime.span, lifetime.id, Node::Lifetime(lifetime)); } fn visit_vis(&mut self, visibility: &'hir Visibility) { @@ -450,7 +460,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { VisibilityKind::Crate(_) | VisibilityKind::Inherited => {} VisibilityKind::Restricted { id, .. } => { - self.insert(id, Node::Visibility(visibility)); + self.insert(visibility.span, id, Node::Visibility(visibility)); self.with_parent(id, |this| { intravisit::walk_vis(this, visibility); }); @@ -462,20 +472,20 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { let def_index = self.definitions.opt_def_index(macro_def.id).unwrap(); self.with_dep_node_owner(def_index, macro_def, |this| { - this.insert(macro_def.id, Node::MacroDef(macro_def)); + this.insert(macro_def.span, macro_def.id, Node::MacroDef(macro_def)); }); } fn visit_variant(&mut self, v: &'hir Variant, g: &'hir Generics, item_id: NodeId) { let id = v.node.data.id(); - self.insert(id, Node::Variant(v)); + self.insert(v.span, id, Node::Variant(v)); self.with_parent(id, |this| { intravisit::walk_variant(this, v, g, item_id); }); } fn visit_struct_field(&mut self, field: &'hir StructField) { - self.insert(field.id, Node::Field(field)); + self.insert(field.span, field.id, Node::Field(field)); self.with_parent(field.id, |this| { intravisit::walk_struct_field(this, field); }); diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index cf7a7abf95a6c..ef777abfbc41a 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -1032,14 +1032,14 @@ pub fn map_crate<'hir>(sess: &::session::Session, let mut collector = NodeCollector::root(&forest.krate, &forest.dep_graph, &definitions, - hcx); + hcx, + sess.source_map()); intravisit::walk_crate(&mut collector, &forest.krate); let crate_disambiguator = sess.local_crate_disambiguator(); let cmdline_args = sess.opts.dep_tracking_hash(); collector.finalize_and_compute_crate_hash(crate_disambiguator, cstore, - sess.source_map(), cmdline_args) }; From 40f80940035af4b44aa7846ed34abee71151d3ee Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 13:35:54 -0500 Subject: [PATCH 02/10] add some `debug!` into lowering --- src/librustc/hir/lowering.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 7ac3b033437fa..e4819d6fb1dae 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1866,6 +1866,10 @@ impl<'a> LoweringContext<'a> { } else { self.lower_node_id(segment.id) }; + debug!( + "lower_path_segment: ident={:?} original-id={:?} new-id={:?}", + segment.ident, segment.id, id, + ); hir::PathSegment::new( segment.ident, @@ -2955,6 +2959,9 @@ impl<'a> LoweringContext<'a> { name: &mut Name, attrs: &hir::HirVec, ) -> hir::ItemKind { + debug!("lower_use_tree(tree={:?})", tree); + debug!("lower_use_tree: vis = {:?}", vis); + let path = &tree.prefix; let segments = prefix .segments @@ -4540,6 +4547,7 @@ impl<'a> LoweringContext<'a> { VisibilityKind::Public => hir::VisibilityKind::Public, VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar), VisibilityKind::Restricted { ref path, id } => { + debug!("lower_visibility: restricted path id = {:?}", id); let lowered_id = if let Some(owner) = explicit_owner { self.lower_node_id_with_owner(id, owner) } else { From a0a47904d64031a7a24828d6ad368ea9da33082a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 13:36:11 -0500 Subject: [PATCH 03/10] renumber segment ids for visibilities whenever we clone them --- src/librustc/hir/lowering.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index e4819d6fb1dae..79d605c0035ed 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3029,12 +3029,7 @@ impl<'a> LoweringContext<'a> { hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited, hir::VisibilityKind::Restricted { ref path, id: _, hir_id: _ } => { let id = this.next_id(); - let mut path = path.clone(); - for seg in path.segments.iter_mut() { - if seg.id.is_some() { - seg.id = Some(this.next_id().node_id); - } - } + let path = this.renumber_segment_ids(path); hir::VisibilityKind::Restricted { path, id: id.node_id, @@ -3119,8 +3114,9 @@ impl<'a> LoweringContext<'a> { hir::VisibilityKind::Inherited => hir::VisibilityKind::Inherited, hir::VisibilityKind::Restricted { ref path, id: _, hir_id: _ } => { let id = this.next_id(); + let path = this.renumber_segment_ids(path); hir::VisibilityKind::Restricted { - path: path.clone(), + path: path, id: id.node_id, hir_id: id.hir_id, } @@ -3154,6 +3150,20 @@ impl<'a> LoweringContext<'a> { } } + /// Paths like the visibility path in `pub(super) use foo::{bar, baz}` are repeated + /// many times in the HIR tree; for each occurrence, we need to assign distinct + /// node-ids. (See e.g. #56128.) + fn renumber_segment_ids(&mut self, path: &P) -> P { + debug!("renumber_segment_ids(path = {:?})", path); + let mut path = path.clone(); + for seg in path.segments.iter_mut() { + if seg.id.is_some() { + seg.id = Some(self.next_id().node_id); + } + } + path + } + fn lower_trait_item(&mut self, i: &TraitItem) -> hir::TraitItem { let LoweredNodeId { node_id, hir_id } = self.lower_node_id(i.id); let trait_item_def_id = self.resolver.definitions().local_def_id(node_id); From 4687eebc287e6f715b237dbb15f6c734715b474a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 15:34:11 -0500 Subject: [PATCH 04/10] preserve the original visibility for the "list stem" node Without this, the `vis` does not wind up in the tree anywhere, and then we get ICEs because the node-ids it refers to are not present. The motivation seemed to be documentation, but `ListStem` HIR nodes are ignored in rustdoc, from what I can tell. --- src/librustc/hir/lowering.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 79d605c0035ed..eec9d084d7e3a 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3139,12 +3139,8 @@ impl<'a> LoweringContext<'a> { }); } - // Privatize the degenerate import base, used only to check - // the stability of `use a::{};`, to avoid it showing up as - // a re-export by accident when `pub`, e.g. in documentation. let def = self.expect_full_def_from_use(id).next().unwrap_or(Def::Err); let path = P(self.lower_path_extra(def, &prefix, ParamMode::Explicit, None)); - *vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityKind::Inherited); hir::ItemKind::Use(path, hir::UseKind::ListStem) } } From 2bd2fc9418ee986fd96c32308dc2ef0efcc7973d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 14:20:03 -0500 Subject: [PATCH 05/10] add regression test --- src/test/ui/issues/issue-56128.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/test/ui/issues/issue-56128.rs diff --git a/src/test/ui/issues/issue-56128.rs b/src/test/ui/issues/issue-56128.rs new file mode 100644 index 0000000000000..4d36fec9c04fa --- /dev/null +++ b/src/test/ui/issues/issue-56128.rs @@ -0,0 +1,13 @@ +// Regression test for #56128. When this `pub(super) use...` gets +// exploded in the HIR, we were not handling ids correctly. + +mod bar { + pub(super) use self::baz::{x, y}; + + mod baz { + pub fn x() { } + pub fn y() { } + } +} + +fn main() { } From 4c7ce7c8973ff8f4b9427faffff3ceda8a1ab7f2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 14:24:36 -0500 Subject: [PATCH 06/10] pass vis by shared reference We are not mutating it now. --- src/librustc/hir/lowering.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index eec9d084d7e3a..5b38263f90f0c 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2752,7 +2752,7 @@ impl<'a> LoweringContext<'a> { id: NodeId, name: &mut Name, attrs: &hir::HirVec, - vis: &mut hir::Visibility, + vis: &hir::Visibility, i: &ItemKind, ) -> hir::ItemKind { match *i { @@ -2955,7 +2955,7 @@ impl<'a> LoweringContext<'a> { tree: &UseTree, prefix: &Path, id: NodeId, - vis: &mut hir::Visibility, + vis: &hir::Visibility, name: &mut Name, attrs: &hir::HirVec, ) -> hir::ItemKind { @@ -3086,7 +3086,7 @@ impl<'a> LoweringContext<'a> { hir_id: new_hir_id, } = self.lower_node_id(id); - let mut vis = vis.clone(); + let vis = vis.clone(); let mut name = name.clone(); let mut prefix = prefix.clone(); @@ -3104,7 +3104,7 @@ impl<'a> LoweringContext<'a> { let item = this.lower_use_tree(use_tree, &prefix, new_id, - &mut vis, + &vis, &mut name, attrs); @@ -3384,7 +3384,7 @@ impl<'a> LoweringContext<'a> { pub fn lower_item(&mut self, i: &Item) -> Option { let mut name = i.ident.name; - let mut vis = self.lower_visibility(&i.vis, None); + let vis = self.lower_visibility(&i.vis, None); let attrs = self.lower_attrs(&i.attrs); if let ItemKind::MacroDef(ref def) = i.node { if !def.legacy || attr::contains_name(&i.attrs, "macro_export") || @@ -3403,7 +3403,7 @@ impl<'a> LoweringContext<'a> { return None; } - let node = self.lower_item_kind(i.id, &mut name, &attrs, &mut vis, &i.node); + let node = self.lower_item_kind(i.id, &mut name, &attrs, &vis, &i.node); let LoweredNodeId { node_id, hir_id } = self.lower_node_id(i.id); From 9cdf4911db1b143f0e40913793389c72e4fa6b17 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 16:09:17 -0500 Subject: [PATCH 07/10] hack: ignore list-stems for pub lint --- src/librustc_lint/builtin.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 7dd1ca3493e9d..0348ba1f1afec 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1136,7 +1136,15 @@ impl UnreachablePub { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub { fn check_item(&mut self, cx: &LateContext, item: &hir::Item) { - self.perform_lint(cx, "item", item.id, &item.vis, item.span, true); + match item.node { + hir::ItemKind::Use(_, hir::UseKind::ListStem) => { + // Hack: ignore these `use foo::{}` remnants which are just a figment + // our IR. + } + _ => { + self.perform_lint(cx, "item", item.id, &item.vis, item.span, true); + } + } } fn check_foreign_item(&mut self, cx: &LateContext, foreign_item: &hir::ForeignItem) { From ebf3c8d8e993ea0ef94c9ff9bf33436c6a526e19 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Nov 2018 18:50:10 -0500 Subject: [PATCH 08/10] add compile-pass annotation --- src/test/ui/issues/issue-56128.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/ui/issues/issue-56128.rs b/src/test/ui/issues/issue-56128.rs index 4d36fec9c04fa..3a3eccdc33ce8 100644 --- a/src/test/ui/issues/issue-56128.rs +++ b/src/test/ui/issues/issue-56128.rs @@ -1,5 +1,7 @@ // Regression test for #56128. When this `pub(super) use...` gets // exploded in the HIR, we were not handling ids correctly. +// +// compile-pass mod bar { pub(super) use self::baz::{x, y}; From b83150e6acb2d80a5885d99cf0fca29b2f2fc513 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 22 Nov 2018 06:41:26 -0500 Subject: [PATCH 09/10] only reset non-restricted visibilities --- src/librustc/hir/lowering.rs | 33 +++++++++++++++++++++++++++------ src/librustc_lint/builtin.rs | 10 +--------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 5b38263f90f0c..e53deaf87881c 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2752,7 +2752,7 @@ impl<'a> LoweringContext<'a> { id: NodeId, name: &mut Name, attrs: &hir::HirVec, - vis: &hir::Visibility, + vis: &mut hir::Visibility, i: &ItemKind, ) -> hir::ItemKind { match *i { @@ -2955,7 +2955,7 @@ impl<'a> LoweringContext<'a> { tree: &UseTree, prefix: &Path, id: NodeId, - vis: &hir::Visibility, + vis: &mut hir::Visibility, name: &mut Name, attrs: &hir::HirVec, ) -> hir::ItemKind { @@ -3086,7 +3086,7 @@ impl<'a> LoweringContext<'a> { hir_id: new_hir_id, } = self.lower_node_id(id); - let vis = vis.clone(); + let mut vis = vis.clone(); let mut name = name.clone(); let mut prefix = prefix.clone(); @@ -3104,7 +3104,7 @@ impl<'a> LoweringContext<'a> { let item = this.lower_use_tree(use_tree, &prefix, new_id, - &vis, + &mut vis, &mut name, attrs); @@ -3139,6 +3139,27 @@ impl<'a> LoweringContext<'a> { }); } + // Subtle and a bit hacky: we lower the privacy level + // of the list stem to "private" most of the time, but + // not for "restricted" paths. The key thing is that + // we don't want it to stay as `pub` (with no caveats) + // because that affects rustdoc and also the lints + // about `pub` items. But we can't *always* make it + // private -- particularly not for restricted paths -- + // because it contains node-ids that would then be + // unused, failing the check that HirIds are "densely + // assigned". + match vis.node { + hir::VisibilityKind::Public | + hir::VisibilityKind::Crate(_) | + hir::VisibilityKind::Inherited => { + *vis = respan(prefix.span.shrink_to_lo(), hir::VisibilityKind::Inherited); + } + hir::VisibilityKind::Restricted { .. } => { + // do nothing here, as described in the comment on the match + } + } + let def = self.expect_full_def_from_use(id).next().unwrap_or(Def::Err); let path = P(self.lower_path_extra(def, &prefix, ParamMode::Explicit, None)); hir::ItemKind::Use(path, hir::UseKind::ListStem) @@ -3384,7 +3405,7 @@ impl<'a> LoweringContext<'a> { pub fn lower_item(&mut self, i: &Item) -> Option { let mut name = i.ident.name; - let vis = self.lower_visibility(&i.vis, None); + let mut vis = self.lower_visibility(&i.vis, None); let attrs = self.lower_attrs(&i.attrs); if let ItemKind::MacroDef(ref def) = i.node { if !def.legacy || attr::contains_name(&i.attrs, "macro_export") || @@ -3403,7 +3424,7 @@ impl<'a> LoweringContext<'a> { return None; } - let node = self.lower_item_kind(i.id, &mut name, &attrs, &vis, &i.node); + let node = self.lower_item_kind(i.id, &mut name, &attrs, &mut vis, &i.node); let LoweredNodeId { node_id, hir_id } = self.lower_node_id(i.id); diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 0348ba1f1afec..7dd1ca3493e9d 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1136,15 +1136,7 @@ impl UnreachablePub { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub { fn check_item(&mut self, cx: &LateContext, item: &hir::Item) { - match item.node { - hir::ItemKind::Use(_, hir::UseKind::ListStem) => { - // Hack: ignore these `use foo::{}` remnants which are just a figment - // our IR. - } - _ => { - self.perform_lint(cx, "item", item.id, &item.vis, item.span, true); - } - } + self.perform_lint(cx, "item", item.id, &item.vis, item.span, true); } fn check_foreign_item(&mut self, cx: &LateContext, foreign_item: &hir::ForeignItem) { From 5f2a173f75204e23737eef128edc74f88dba7f39 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 22 Nov 2018 06:48:13 -0500 Subject: [PATCH 10/10] explain how this works --- src/librustc/hir/lowering.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index e53deaf87881c..b3ba2968c9f51 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3070,7 +3070,29 @@ impl<'a> LoweringContext<'a> { hir::ItemKind::Use(path, hir::UseKind::Glob) } UseTreeKind::Nested(ref trees) => { - // Nested imports are desugared into simple imports. + // Nested imports are desugared into simple + // imports. So if we start with + // + // ``` + // pub(x) use foo::{a, b}; + // ``` + // + // we will create three items: + // + // ``` + // pub(x) use foo::a; + // pub(x) use foo::b; + // pub(x) use foo::{}; // <-- this is called the `ListStem` + // ``` + // + // The first two are produced by recursively invoking + // `lower_use_tree` (and indeed there may be things + // like `use foo::{a::{b, c}}` and so forth). They + // wind up being directly added to + // `self.items`. However, the structure of this + // function also requires us to return one item, and + // for that we return the `{}` import (called the + // "`ListStem`"). let prefix = Path { segments,