Skip to content

Commit 41f1e18

Browse files
jseyfriedjseyfried
committed
Use DefIds instead of NodeIds for pub(restricted) visibilities.
1 parent 467a7f0 commit 41f1e18

File tree

12 files changed

+137
-161
lines changed

12 files changed

+137
-161
lines changed

‎src/librustc/ty/mod.rs‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -219,63 +219,65 @@ pub enum Visibility {
219219
/// Visible everywhere (including in other crates).
220220
Public,
221221
/// Visible only in the given crate-local module.
222-
Restricted(NodeId),
222+
Restricted(DefId),
223223
/// Not visible anywhere in the local crate. This is the visibility of private external items.
224-
PrivateExternal,
224+
Invisible,
225225
}
226226

227-
pub trait NodeIdTree {
228-
fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool;
227+
pub trait DefIdTree: Copy {
228+
fn parent(self, id: DefId) -> Option<DefId>;
229229
}
230230

231-
impl<'a> NodeIdTree for ast_map::Map<'a> {
232-
fn is_descendant_of(&self, node: NodeId, ancestor: NodeId) -> bool {
233-
let mut node_ancestor = node;
234-
while node_ancestor != ancestor {
235-
let node_ancestor_parent = self.get_module_parent(node_ancestor);
236-
if node_ancestor_parent == node_ancestor {
237-
return false;
238-
}
239-
node_ancestor = node_ancestor_parent;
240-
}
241-
true
231+
impl<'a, 'gcx, 'tcx> DefIdTree for TyCtxt<'a, 'gcx, 'tcx> {
232+
fn parent(self, id: DefId) -> Option<DefId> {
233+
self.def_key(id).parent.map(|index| DefId { index: index, ..id })
242234
}
243235
}
244236

245237
impl Visibility {
246238
pub fn from_hir(visibility: &hir::Visibility, id: NodeId, tcx: TyCtxt) -> Self {
247239
match *visibility {
248240
hir::Public => Visibility::Public,
249-
hir::Visibility::Crate => Visibility::Restricted(ast::CRATE_NODE_ID),
241+
hir::Visibility::Crate => Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)),
250242
hir::Visibility::Restricted { ref path, .. } => match path.def {
251243
// If there is no resolution, `resolve` will have already reported an error, so
252244
// assume that the visibility is public to avoid reporting more privacy errors.
253245
Def::Err => Visibility::Public,
254-
def => Visibility::Restricted(tcx.map.as_local_node_id(def.def_id()).unwrap()),
246+
def => Visibility::Restricted(def.def_id()),
255247
},
256-
hir::Inherited => Visibility::Restricted(tcx.map.get_module_parent(id)),
248+
hir::Inherited => {
249+
Visibility::Restricted(tcx.map.local_def_id(tcx.map.get_module_parent(id)))
250+
}
257251
}
258252
}
259253

260254
/// Returns true if an item with this visibility is accessible from the given block.
261-
pub fn is_accessible_from<T: NodeIdTree>(self, block: NodeId, tree: &T) -> bool {
255+
pub fn is_accessible_from<T: DefIdTree>(self, mut module: DefId, tree: T) -> bool {
262256
let restriction = match self {
263257
// Public items are visible everywhere.
264258
Visibility::Public => return true,
265259
// Private items from other crates are visible nowhere.
266-
Visibility::PrivateExternal => return false,
260+
Visibility::Invisible => return false,
267261
// Restricted items are visible in an arbitrary local module.
262+
Visibility::Restricted(other) if other.krate != module.krate => return false,
268263
Visibility::Restricted(module) => module,
269264
};
270265

271-
tree.is_descendant_of(block, restriction)
266+
while module != restriction {
267+
match tree.parent(module) {
268+
Some(parent) => module = parent,
269+
None => return false,
270+
}
271+
}
272+
273+
true
272274
}
273275

274276
/// Returns true if this visibility is at least as accessible as the given visibility
275-
pub fn is_at_least<T: NodeIdTree>(self, vis: Visibility, tree: &T) -> bool {
277+
pub fn is_at_least<T: DefIdTree>(self, vis: Visibility, tree: T) -> bool {
276278
let vis_restriction = match vis {
277279
Visibility::Public => return self == Visibility::Public,
278-
Visibility::PrivateExternal => return true,
280+
Visibility::Invisible => return true,
279281
Visibility::Restricted(module) => module,
280282
};
281283

@@ -1779,7 +1781,7 @@ impl<'a, 'gcx, 'tcx> FieldDef {
17791781
block: Option<NodeId>,
17801782
tcx: TyCtxt<'a, 'gcx, 'tcx>,
17811783
substs: &'tcx Substs<'tcx>) -> bool {
1782-
block.map_or(true, |b| self.vis.is_accessible_from(b, &tcx.map)) &&
1784+
block.map_or(true, |b| tcx.vis_is_accessible_from(self.vis, b)) &&
17831785
self.ty(tcx, substs).is_uninhabited_recurse(visited, block, tcx)
17841786
}
17851787
}
@@ -2266,6 +2268,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
22662268
}
22672269
}
22682270

2271+
pub fn vis_is_accessible_from(self, vis: Visibility, block: NodeId) -> bool {
2272+
vis.is_accessible_from(self.map.local_def_id(self.map.get_module_parent(block)), self)
2273+
}
2274+
22692275
pub fn item_name(self, id: DefId) -> ast::Name {
22702276
if let Some(id) = self.map.as_local_node_id(id) {
22712277
self.map.name(id)

‎src/librustc_metadata/decoder.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ impl<'a, 'tcx> CrateMetadata {
588588
ty::FieldDef {
589589
did: self.local_def_id(index),
590590
name: self.item_name(index),
591-
vis: f.visibility
591+
vis: f.visibility.decode(self)
592592
}
593593
}).collect(),
594594
disr_val: ConstInt::Infer(data.disr),
@@ -678,7 +678,7 @@ impl<'a, 'tcx> CrateMetadata {
678678
pub fn get_visibility(&self, id: DefIndex) -> ty::Visibility {
679679
match self.is_proc_macro(id) {
680680
true => ty::Visibility::Public,
681-
false => self.entry(id).visibility,
681+
false => self.entry(id).visibility.decode(self),
682682
}
683683
}
684684

@@ -885,7 +885,7 @@ impl<'a, 'tcx> CrateMetadata {
885885
ty::AssociatedItem {
886886
name: name,
887887
kind: ty::AssociatedKind::Const,
888-
vis: item.visibility,
888+
vis: item.visibility.decode(self),
889889
defaultness: container.defaultness(),
890890
def_id: self.local_def_id(id),
891891
container: container.with_def_id(parent),
@@ -898,7 +898,7 @@ impl<'a, 'tcx> CrateMetadata {
898898
ty::AssociatedItem {
899899
name: name,
900900
kind: ty::AssociatedKind::Method,
901-
vis: item.visibility,
901+
vis: item.visibility.decode(self),
902902
defaultness: data.container.defaultness(),
903903
def_id: self.local_def_id(id),
904904
container: data.container.with_def_id(parent),
@@ -910,7 +910,7 @@ impl<'a, 'tcx> CrateMetadata {
910910
ty::AssociatedItem {
911911
name: name,
912912
kind: ty::AssociatedKind::Type,
913-
vis: item.visibility,
913+
vis: item.visibility.decode(self),
914914
defaultness: container.defaultness(),
915915
def_id: self.local_def_id(id),
916916
container: container.with_def_id(parent),

‎src/librustc_metadata/encoder.rs‎

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
268268

269269
Entry {
270270
kind: EntryKind::Variant(self.lazy(&data)),
271-
visibility: enum_vis.simplify(),
271+
visibility: self.lazy(&ty::Visibility::from_hir(enum_vis, enum_id, tcx)),
272272
span: self.lazy(&tcx.def_span(def_id)),
273273
attributes: self.encode_attributes(&tcx.get_attrs(def_id)),
274274
children: self.lazy_seq(variant.fields.iter().map(|f| {
@@ -306,7 +306,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
306306

307307
Entry {
308308
kind: EntryKind::Mod(self.lazy(&data)),
309-
visibility: vis.simplify(),
309+
visibility: self.lazy(&ty::Visibility::from_hir(vis, id, tcx)),
310310
span: self.lazy(&md.inner),
311311
attributes: self.encode_attributes(attrs),
312312
children: self.lazy_seq(md.item_ids.iter().map(|item_id| {
@@ -327,30 +327,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
327327
}
328328
}
329329

330-
trait Visibility {
331-
fn simplify(&self) -> ty::Visibility;
332-
}
333-
334-
impl Visibility for hir::Visibility {
335-
fn simplify(&self) -> ty::Visibility {
336-
if *self == hir::Public {
337-
ty::Visibility::Public
338-
} else {
339-
ty::Visibility::PrivateExternal
340-
}
341-
}
342-
}
343-
344-
impl Visibility for ty::Visibility {
345-
fn simplify(&self) -> ty::Visibility {
346-
if *self == ty::Visibility::Public {
347-
ty::Visibility::Public
348-
} else {
349-
ty::Visibility::PrivateExternal
350-
}
351-
}
352-
}
353-
354330
impl<'a, 'b, 'tcx> IndexBuilder<'a, 'b, 'tcx> {
355331
fn encode_fields(&mut self, adt_def_id: DefId) {
356332
let def = self.tcx.lookup_adt_def(adt_def_id);
@@ -386,7 +362,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
386362

387363
Entry {
388364
kind: EntryKind::Field,
389-
visibility: field.vis.simplify(),
365+
visibility: self.lazy(&field.vis),
390366
span: self.lazy(&tcx.def_span(def_id)),
391367
attributes: self.encode_attributes(&variant_data.fields()[field_index].attrs),
392368
children: LazySeq::empty(),
@@ -419,7 +395,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
419395

420396
Entry {
421397
kind: EntryKind::Struct(self.lazy(&data)),
422-
visibility: struct_vis.simplify(),
398+
visibility: self.lazy(&ty::Visibility::from_hir(struct_vis, struct_id, tcx)),
423399
span: self.lazy(&tcx.def_span(def_id)),
424400
attributes: LazySeq::empty(),
425401
children: LazySeq::empty(),
@@ -485,7 +461,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
485461

486462
Entry {
487463
kind: kind,
488-
visibility: trait_item.vis.simplify(),
464+
visibility: self.lazy(&trait_item.vis),
489465
span: self.lazy(&ast_item.span),
490466
attributes: self.encode_attributes(&ast_item.attrs),
491467
children: LazySeq::empty(),
@@ -574,7 +550,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
574550

575551
Entry {
576552
kind: kind,
577-
visibility: impl_item.vis.simplify(),
553+
visibility: self.lazy(&impl_item.vis),
578554
span: self.lazy(&ast_item.span),
579555
attributes: self.encode_attributes(&ast_item.attrs),
580556
children: LazySeq::empty(),
@@ -736,7 +712,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
736712

737713
Entry {
738714
kind: kind,
739-
visibility: item.vis.simplify(),
715+
visibility: self.lazy(&ty::Visibility::from_hir(&item.vis, item.id, tcx)),
740716
span: self.lazy(&item.span),
741717
attributes: self.encode_attributes(&item.attrs),
742718
children: match item.node {
@@ -849,7 +825,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
849825
kind: EntryKind::MacroDef(self.lazy(&MacroDef {
850826
body: ::syntax::print::pprust::tts_to_string(&macro_def.body)
851827
})),
852-
visibility: ty::Visibility::Public,
828+
visibility: self.lazy(&ty::Visibility::Public),
853829
span: self.lazy(&macro_def.span),
854830

855831
attributes: self.encode_attributes(&macro_def.attrs),
@@ -950,7 +926,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
950926

951927
Entry {
952928
kind: kind,
953-
visibility: nitem.vis.simplify(),
929+
visibility: self.lazy(&ty::Visibility::from_hir(&nitem.vis, nitem.id, tcx)),
954930
span: self.lazy(&nitem.span),
955931
attributes: self.encode_attributes(&nitem.attrs),
956932
children: LazySeq::empty(),
@@ -1032,7 +1008,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
10321008
let tcx = self.tcx;
10331009
Entry {
10341010
kind: EntryKind::Type,
1035-
visibility: ty::Visibility::Public,
1011+
visibility: self.lazy(&ty::Visibility::Public),
10361012
span: self.lazy(&tcx.def_span(def_id)),
10371013
attributes: LazySeq::empty(),
10381014
children: LazySeq::empty(),
@@ -1060,7 +1036,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
10601036

10611037
Entry {
10621038
kind: EntryKind::Closure(self.lazy(&data)),
1063-
visibility: ty::Visibility::Public,
1039+
visibility: self.lazy(&ty::Visibility::Public),
10641040
span: self.lazy(&tcx.def_span(def_id)),
10651041
attributes: self.encode_attributes(&tcx.get_attrs(def_id)),
10661042
children: LazySeq::empty(),

‎src/librustc_metadata/schema.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ pub struct TraitImpls {
201201
#[derive(RustcEncodable, RustcDecodable)]
202202
pub struct Entry<'tcx> {
203203
pub kind: EntryKind<'tcx>,
204-
pub visibility: ty::Visibility,
204+
pub visibility: Lazy<ty::Visibility>,
205205
pub span: Lazy<Span>,
206206
pub attributes: LazySeq<ast::Attribute>,
207207
pub children: LazySeq<DefIndex>,

0 commit comments

Comments
 (0)