Skip to content

Commit 88fcf83

Browse files
committed
rustc_resolve: don't handle impl items as if they were modules.
1 parent 94fc4f2 commit 88fcf83

24 files changed

+158
-364
lines changed

src/librustc/middle/ty.rs

+4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ use middle::dependency_format;
5050
use middle::lang_items::{FnTraitLangItem, FnMutTraitLangItem};
5151
use middle::lang_items::{FnOnceTraitLangItem, TyDescStructLangItem};
5252
use middle::mem_categorization as mc;
53+
use middle::privacy::LastPrivateMap;
5354
use middle::region;
5455
use middle::resolve_lifetime;
5556
use middle::infer;
@@ -679,6 +680,7 @@ pub struct ctxt<'tcx> {
679680
pub sess: Session,
680681
pub def_map: DefMap,
681682
pub partial_def_map: PartialDefMap,
683+
pub last_private_map: RefCell<LastPrivateMap>,
682684

683685
pub named_region_map: resolve_lifetime::NamedRegionMap,
684686

@@ -2335,6 +2337,7 @@ pub fn mk_ctxt<'tcx>(s: Session,
23352337
arenas: &'tcx CtxtArenas<'tcx>,
23362338
def_map: DefMap,
23372339
partial_def_map: PartialDefMap,
2340+
last_private_map: LastPrivateMap,
23382341
named_region_map: resolve_lifetime::NamedRegionMap,
23392342
map: ast_map::Map<'tcx>,
23402343
freevars: RefCell<FreevarMap>,
@@ -2358,6 +2361,7 @@ pub fn mk_ctxt<'tcx>(s: Session,
23582361
sess: s,
23592362
def_map: def_map,
23602363
partial_def_map: partial_def_map,
2364+
last_private_map: RefCell::new(last_private_map),
23612365
region_maps: region_maps,
23622366
node_types: RefCell::new(FnvHashMap()),
23632367
item_substs: RefCell::new(NodeMap()),

src/librustc_driver/driver.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
603603
arenas,
604604
def_map,
605605
partial_def_map,
606+
last_private_map,
606607
named_region_map,
607608
ast_map,
608609
freevars,
@@ -623,10 +624,9 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
623624
time(time_passes, "const checking", (), |_|
624625
middle::check_const::check_crate(&ty_cx));
625626

626-
let maps = (external_exports, last_private_map);
627627
let (exported_items, public_items) =
628-
time(time_passes, "privacy checking", maps, |(a, b)|
629-
rustc_privacy::check_crate(&ty_cx, &export_map, a, b));
628+
time(time_passes, "privacy checking", (), |_|
629+
rustc_privacy::check_crate(&ty_cx, &export_map, external_exports));
630630

631631
// Do not move this check past lint
632632
time(time_passes, "stability index", (), |_|

src/librustc_privacy/lib.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ use rustc::middle::def;
3838
use rustc::middle::privacy::ImportUse::*;
3939
use rustc::middle::privacy::LastPrivate::*;
4040
use rustc::middle::privacy::PrivateDep::*;
41-
use rustc::middle::privacy::{ExportedItems, PublicItems, LastPrivateMap};
42-
use rustc::middle::privacy::{ExternalExports};
41+
use rustc::middle::privacy::{ExternalExports, ExportedItems, PublicItems};
4342
use rustc::middle::ty::{MethodTypeParam, MethodStatic};
4443
use rustc::middle::ty::{MethodCall, MethodMap, MethodOrigin, MethodParam};
4544
use rustc::middle::ty::{MethodStaticClosure, MethodObject};
@@ -379,7 +378,6 @@ struct PrivacyVisitor<'a, 'tcx: 'a> {
379378
in_foreign: bool,
380379
parents: NodeMap<ast::NodeId>,
381380
external_exports: ExternalExports,
382-
last_private_map: LastPrivateMap,
383381
}
384382

385383
enum PrivacyResult {
@@ -730,7 +728,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> {
730728
&format!("{} `{}`", tyname, name)[])
731729
};
732730

733-
match self.last_private_map[path_id] {
731+
match self.tcx.last_private_map.borrow()[path_id] {
734732
LastMod(AllPublic) => {},
735733
LastMod(DependsOn(def)) => {
736734
self.report_error(ck_public(def));
@@ -1499,8 +1497,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
14991497

15001498
pub fn check_crate(tcx: &ty::ctxt,
15011499
export_map: &def::ExportMap,
1502-
external_exports: ExternalExports,
1503-
last_private_map: LastPrivateMap)
1500+
external_exports: ExternalExports)
15041501
-> (ExportedItems, PublicItems) {
15051502
let krate = tcx.map.krate();
15061503

@@ -1518,7 +1515,6 @@ pub fn check_crate(tcx: &ty::ctxt,
15181515
tcx: tcx,
15191516
parents: visitor.parents,
15201517
external_exports: external_exports,
1521-
last_private_map: last_private_map,
15221518
};
15231519
visit::walk_crate(&mut visitor, krate);
15241520

src/librustc_resolve/build_reduced_graph.rs

+10-228
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,16 @@ use syntax::ast::{ForeignItem, ForeignItemFn, ForeignItemStatic};
3939
use syntax::ast::{Item, ItemConst, ItemEnum, ItemExternCrate, ItemFn};
4040
use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic};
4141
use syntax::ast::{ItemStruct, ItemTrait, ItemTy, ItemUse};
42-
use syntax::ast::{MethodImplItem, Name, NamedField, NodeId};
42+
use syntax::ast::{Name, NamedField, NodeId};
4343
use syntax::ast::{PathListIdent, PathListMod, Public};
4444
use syntax::ast::StmtDecl;
4545
use syntax::ast::StructVariantKind;
4646
use syntax::ast::TupleVariantKind;
47-
use syntax::ast::TyObjectSum;
48-
use syntax::ast::{TypeImplItem, UnnamedField};
47+
use syntax::ast::UnnamedField;
4948
use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
5049
use syntax::ast::{Visibility};
51-
use syntax::ast::TyPath;
5250
use syntax::ast;
53-
use syntax::ast_util::{self, PostExpansionMethod, local_def};
51+
use syntax::ast_util::{self, local_def};
5452
use syntax::attr::AttrMetaMethods;
5553
use syntax::parse::token::{self, special_idents};
5654
use syntax::codemap::{Span, DUMMY_SP};
@@ -177,12 +175,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
177175
Some(TypeNS)
178176
}
179177
ForbidDuplicateTypesAndModules => {
180-
match child.def_for_namespace(TypeNS) {
181-
None => {}
182-
Some(_) if child.get_module_if_available()
183-
.map(|m| m.kind.get()) ==
184-
Some(ImplModuleKind) => {}
185-
Some(_) => duplicate_type = TypeError
178+
if child.defined_in_namespace(TypeNS) {
179+
duplicate_type = TypeError;
186180
}
187181
Some(TypeNS)
188182
}
@@ -461,9 +455,6 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
461455
name_bindings.define_type(DefTy(local_def(item.id), true), sp, modifiers);
462456

463457
let parent_link = self.get_parent_link(parent, name);
464-
// We want to make sure the module type is EnumModuleKind
465-
// even if there's already an ImplModuleKind module defined,
466-
// since that's how we prevent duplicate enum definitions
467458
name_bindings.set_module_kind(parent_link,
468459
Some(local_def(item.id)),
469460
EnumModuleKind,
@@ -513,132 +504,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
513504
parent.clone()
514505
}
515506

516-
ItemImpl(_, _, _, None, ref ty, ref impl_items) => {
517-
// If this implements an anonymous trait, then add all the
518-
// methods within to a new module, if the type was defined
519-
// within this module.
520-
521-
let mod_name = match ty.node {
522-
TyPath(ref path) if path.segments.len() == 1 => {
523-
// FIXME(18446) we should distinguish between the name of
524-
// a trait and the name of an impl of that trait.
525-
Some(path.segments.last().unwrap().identifier.name)
526-
}
527-
TyObjectSum(ref lhs_ty, _) => {
528-
match lhs_ty.node {
529-
TyPath(ref path) if path.segments.len() == 1 => {
530-
Some(path.segments.last().unwrap().identifier.name)
531-
}
532-
_ => {
533-
None
534-
}
535-
}
536-
}
537-
_ => {
538-
None
539-
}
540-
};
541-
542-
let mod_name = match mod_name {
543-
Some(mod_name) => mod_name,
544-
None => {
545-
self.resolve_error(ty.span,
546-
"inherent implementations may \
547-
only be implemented in the same \
548-
module as the type they are \
549-
implemented for");
550-
return parent.clone();
551-
}
552-
};
553-
// Create the module and add all methods.
554-
let child_opt = parent.children.borrow().get(&mod_name)
555-
.and_then(|m| m.get_module_if_available());
556-
let new_parent = match child_opt {
557-
// It already exists
558-
Some(ref child) if (child.kind.get() == ImplModuleKind ||
559-
child.kind.get() == TraitModuleKind) => {
560-
child.clone()
561-
}
562-
Some(ref child) if child.kind.get() == EnumModuleKind ||
563-
child.kind.get() == TypeModuleKind => {
564-
child.clone()
565-
}
566-
// Create the module
567-
_ => {
568-
let name_bindings =
569-
self.add_child(mod_name, parent, ForbidDuplicateModules, sp);
570-
571-
let parent_link = self.get_parent_link(parent, name);
572-
let def_id = local_def(item.id);
573-
let ns = TypeNS;
574-
let is_public =
575-
!name_bindings.defined_in_namespace(ns) ||
576-
name_bindings.defined_in_public_namespace(ns);
577-
578-
name_bindings.define_module(parent_link,
579-
Some(def_id),
580-
ImplModuleKind,
581-
false,
582-
is_public,
583-
sp);
584-
585-
name_bindings.get_module()
586-
}
587-
};
588-
589-
// For each implementation item...
590-
for impl_item in impl_items {
591-
match *impl_item {
592-
MethodImplItem(ref method) => {
593-
// Add the method to the module.
594-
let name = method.pe_ident().name;
595-
let method_name_bindings =
596-
self.add_child(name,
597-
&new_parent,
598-
ForbidDuplicateValues,
599-
method.span);
600-
let def = DefMethod(local_def(method.id),
601-
FromImpl(local_def(item.id)));
602-
603-
// NB: not IMPORTABLE
604-
let modifiers = if method.pe_vis() == ast::Public {
605-
PUBLIC
606-
} else {
607-
DefModifiers::empty()
608-
};
609-
method_name_bindings.define_value(
610-
def,
611-
method.span,
612-
modifiers);
613-
}
614-
TypeImplItem(ref typedef) => {
615-
// Add the typedef to the module.
616-
let name = typedef.ident.name;
617-
let typedef_name_bindings =
618-
self.add_child(
619-
name,
620-
&new_parent,
621-
ForbidDuplicateTypesAndModules,
622-
typedef.span);
623-
let def = DefAssociatedTy(local_def(item.id),
624-
local_def(typedef.id));
625-
// NB: not IMPORTABLE
626-
let modifiers = if typedef.vis == ast::Public {
627-
PUBLIC
628-
} else {
629-
DefModifiers::empty()
630-
};
631-
typedef_name_bindings.define_type(
632-
def,
633-
typedef.span,
634-
modifiers);
635-
}
636-
}
637-
}
638-
parent.clone()
639-
}
640-
641-
ItemImpl(_, _, _, Some(_), _, _) => parent.clone(),
507+
ItemImpl(..) => parent.clone(),
642508

643509
ItemTrait(_, _, _, ref items) => {
644510
let name_bindings =
@@ -804,8 +670,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
804670

805671
let kind = match def {
806672
DefTy(_, true) => EnumModuleKind,
807-
DefTy(_, false) => TypeModuleKind,
808-
DefStruct(..) => ImplModuleKind,
673+
DefTy(_, false) | DefStruct(..) => TypeModuleKind,
809674
_ => NormalModuleKind
810675
};
811676

@@ -979,92 +844,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
979844
}
980845
}
981846
}
982-
DlImpl(def) => {
983-
match csearch::get_type_name_if_impl(&self.session.cstore, def) {
984-
None => {}
985-
Some(final_name) => {
986-
let methods_opt =
987-
csearch::get_methods_if_impl(&self.session.cstore, def);
988-
match methods_opt {
989-
Some(ref methods) if
990-
methods.len() >= 1 => {
991-
debug!("(building reduced graph for \
992-
external crate) processing \
993-
static methods for type name {}",
994-
token::get_name(final_name));
995-
996-
let child_name_bindings =
997-
self.add_child(
998-
final_name,
999-
root,
1000-
OverwriteDuplicates,
1001-
DUMMY_SP);
1002-
1003-
// Process the static methods. First,
1004-
// create the module.
1005-
let type_module;
1006-
let type_def = child_name_bindings.type_def.borrow().clone();
1007-
match type_def {
1008-
Some(TypeNsDef {
1009-
module_def: Some(module_def),
1010-
..
1011-
}) => {
1012-
// We already have a module. This
1013-
// is OK.
1014-
type_module = module_def;
1015-
1016-
// Mark it as an impl module if
1017-
// necessary.
1018-
type_module.kind.set(ImplModuleKind);
1019-
}
1020-
Some(_) | None => {
1021-
let parent_link =
1022-
self.get_parent_link(root, final_name);
1023-
child_name_bindings.define_module(
1024-
parent_link,
1025-
Some(def),
1026-
ImplModuleKind,
1027-
true,
1028-
true,
1029-
DUMMY_SP);
1030-
type_module =
1031-
child_name_bindings.
1032-
get_module();
1033-
}
1034-
}
1035-
1036-
// Add each static method to the module.
1037-
let new_parent = type_module;
1038-
for method_info in methods {
1039-
let name = method_info.name;
1040-
debug!("(building reduced graph for \
1041-
external crate) creating \
1042-
static method '{}'",
1043-
token::get_name(name));
1044-
1045-
let method_name_bindings =
1046-
self.add_child(name,
1047-
&new_parent,
1048-
OverwriteDuplicates,
1049-
DUMMY_SP);
1050-
let def = DefFn(method_info.def_id, false);
1051-
1052-
// NB: not IMPORTABLE
1053-
let modifiers = if method_info.vis == ast::Public {
1054-
PUBLIC
1055-
} else {
1056-
DefModifiers::empty()
1057-
};
1058-
method_name_bindings.define_value(
1059-
def, DUMMY_SP, modifiers);
1060-
}
1061-
}
1062-
1063-
// Otherwise, do nothing.
1064-
Some(_) | None => {}
1065-
}
1066-
}
1067-
}
847+
DlImpl(_) => {
848+
debug!("(building reduced graph for external crate) \
849+
ignoring impl");
1068850
}
1069851
DlField => {
1070852
debug!("(building reduced graph for external crate) \

0 commit comments

Comments
 (0)