Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check uses of Self in impls in the compiler rather than during expansion #23998

Merged
merged 2 commits into from
Apr 8, 2015
Merged

Check uses of Self in impls in the compiler rather than during expansion #23998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc8a8e9
Check uses of `Self` in impls in the compiler rather than during expa…
nrc Apr 3, 2015
63b36ea
Work with assoc types in a super trait.
nrc Apr 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/librustc/middle/astencode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,11 @@ impl tr for def::Def {
def::DefMethod(did, p) => {
def::DefMethod(did.tr(dcx), p.map(|did2| did2.tr(dcx)))
}
def::DefSelfTy(nid) => { def::DefSelfTy(dcx.tr_id(nid)) }
def::DefSelfTy(opt_did, impl_ids) => { def::DefSelfTy(opt_did.map(|did| did.tr(dcx)),
impl_ids.map(|(nid1, nid2)| {
(dcx.tr_id(nid1),
dcx.tr_id(nid2))
})) }
def::DefMod(did) => { def::DefMod(did.tr(dcx)) }
def::DefForeignMod(did) => { def::DefForeignMod(did.tr(dcx)) }
def::DefStatic(did, m) => { def::DefStatic(did.tr(dcx), m) }
Expand Down
12 changes: 7 additions & 5 deletions src/librustc/middle/def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ use std::cell::RefCell;
#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub enum Def {
DefFn(ast::DefId, bool /* is_ctor */),
DefSelfTy(/* trait id */ ast::NodeId),
DefSelfTy(Option<ast::DefId>, // trait id
Option<(ast::NodeId, ast::NodeId)>), // (impl id, self type id)
DefMod(ast::DefId),
DefForeignMod(ast::DefId),
DefStatic(ast::DefId, bool /* is_mutbl */),
Expand Down Expand Up @@ -139,18 +140,19 @@ impl Def {
DefFn(id, _) | DefMod(id) | DefForeignMod(id) | DefStatic(id, _) |
DefVariant(_, id, _) | DefTy(id, _) | DefAssociatedTy(_, id) |
DefTyParam(_, _, id, _) | DefUse(id) | DefStruct(id) | DefTrait(id) |
DefMethod(id, _) | DefConst(id) => {
DefMethod(id, _) | DefConst(id) | DefSelfTy(Some(id), None)=> {
id
}
DefLocal(id) |
DefSelfTy(id) |
DefUpvar(id, _) |
DefRegion(id) |
DefLabel(id) => {
DefLabel(id) |
DefSelfTy(_, Some((_, id))) => {
local_def(id)
}

DefPrimTy(_) => panic!("attempted .def_id() on DefPrimTy")
DefPrimTy(_) => panic!("attempted .def_id() on DefPrimTy"),
DefSelfTy(..) => panic!("attempted .def_id() on invalid DefSelfTy"),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_privacy/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> {
let did = match self.tcx.def_map.borrow().get(&path_id).map(|d| d.full_def()) {
// `int` etc. (None doesn't seem to occur.)
None | Some(def::DefPrimTy(..)) => return false,
Some(def) => def.def_id()
Some(def) => def.def_id(),
};
// A path can only be private if:
// it's in this crate...
Expand Down
169 changes: 90 additions & 79 deletions src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1689,7 +1689,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
}
DefTyParam(..) | DefSelfTy(_) => {
DefTyParam(..) | DefSelfTy(..) => {
for rib in ribs {
match rib.kind {
NormalRibKind | MethodRibKind | ClosureRibKind(..) => {
Expand Down Expand Up @@ -1797,63 +1797,57 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

ItemDefaultImpl(_, ref trait_ref) => {
self.with_optional_trait_ref(Some(trait_ref), |_| {});
self.with_optional_trait_ref(Some(trait_ref), |_, _| {});
}
ItemImpl(_, _,
ItemImpl(_,
_,
ref generics,
ref implemented_traits,
ref opt_trait_ref,
ref self_type,
ref impl_items) => {
self.resolve_implementation(generics,
implemented_traits,
opt_trait_ref,
&**self_type,
item.id,
&impl_items[..]);
}

ItemTrait(_, ref generics, ref bounds, ref trait_items) => {
self.check_if_primitive_type_name(name, item.span);

// Create a new rib for the self type.
let mut self_type_rib = Rib::new(ItemRibKind);

// plain insert (no renaming, types are not currently hygienic....)
let name = special_names::type_self;
self_type_rib.bindings.insert(name, DlDef(DefSelfTy(item.id)));
self.type_ribs.push(self_type_rib);

// Create a new rib for the trait-wide type parameters.
self.with_type_parameter_rib(HasTypeParameters(generics,
TypeSpace,
NormalRibKind),
ItemRibKind),
|this| {
this.visit_generics(generics);
visit::walk_ty_param_bounds_helper(this, bounds);

for trait_item in trait_items {
// Create a new rib for the trait_item-specific type
// parameters.
//
// FIXME #4951: Do we need a node ID here?

let type_parameters = match trait_item.node {
ast::MethodTraitItem(ref sig, _) => {
HasTypeParameters(&sig.generics,
FnSpace,
MethodRibKind)
}
ast::TypeTraitItem(..) => {
this.check_if_primitive_type_name(trait_item.ident.name,
trait_item.span);
NoTypeParameters
}
};
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_trait_item(this, trait_item)
});
}
this.with_self_rib(DefSelfTy(Some(local_def(item.id)), None), |this| {
this.visit_generics(generics);
visit::walk_ty_param_bounds_helper(this, bounds);

for trait_item in trait_items {
// Create a new rib for the trait_item-specific type
// parameters.
//
// FIXME #4951: Do we need a node ID here?

let type_parameters = match trait_item.node {
ast::MethodTraitItem(ref sig, _) => {
HasTypeParameters(&sig.generics,
FnSpace,
MethodRibKind)
}
ast::TypeTraitItem(..) => {
this.check_if_primitive_type_name(trait_item.ident.name,
trait_item.span);
NoTypeParameters
}
};
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_trait_item(this, trait_item)
});
}
});
});

self.type_ribs.pop();
}

ItemMod(_) | ItemForeignMod(_) => {
Expand Down Expand Up @@ -2030,8 +2024,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
visit::walk_generics(self, generics);
}

fn with_current_self_type<T, F>(&mut self, self_type: &Ty, f: F) -> T where
F: FnOnce(&mut Resolver) -> T,
fn with_current_self_type<T, F>(&mut self, self_type: &Ty, f: F) -> T
where F: FnOnce(&mut Resolver) -> T
{
// Handle nested impls (inside fn bodies)
let previous_value = replace(&mut self.current_self_type, Some(self_type.clone()));
Expand All @@ -2044,29 +2038,44 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
opt_trait_ref: Option<&TraitRef>,
f: F)
-> T
where F: FnOnce(&mut Resolver) -> T,
where F: FnOnce(&mut Resolver, Option<DefId>) -> T
{
let mut new_val = None;
let mut new_id = None;
if let Some(trait_ref) = opt_trait_ref {
match self.resolve_trait_reference(trait_ref.ref_id, &trait_ref.path, 0) {
Ok(path_res) => {
self.record_def(trait_ref.ref_id, path_res);
new_val = Some((path_res.base_def.def_id(), trait_ref.clone()));
}
Err(_) => { /* error was already reported */ }
if let Ok(path_res) = self.resolve_trait_reference(trait_ref.ref_id,
&trait_ref.path, 0) {
assert!(path_res.depth == 0);
self.record_def(trait_ref.ref_id, path_res);
new_val = Some((path_res.base_def.def_id(), trait_ref.clone()));
new_id = Some(path_res.base_def.def_id());
}
visit::walk_trait_ref(self, trait_ref);
}
let original_trait_ref = replace(&mut self.current_trait_ref, new_val);
let result = f(self);
let result = f(self, new_id);
self.current_trait_ref = original_trait_ref;
result
}

fn with_self_rib<F>(&mut self, self_def: Def, f: F)
where F: FnOnce(&mut Resolver)
{
let mut self_type_rib = Rib::new(NormalRibKind);

// plain insert (no renaming, types are not currently hygienic....)
let name = special_names::type_self;
self_type_rib.bindings.insert(name, DlDef(self_def));
self.type_ribs.push(self_type_rib);
f(self);
self.type_ribs.pop();
}

fn resolve_implementation(&mut self,
generics: &Generics,
opt_trait_reference: &Option<TraitRef>,
self_type: &Ty,
item_id: NodeId,
impl_items: &[P<ImplItem>]) {
// If applicable, create a rib for the type parameters.
self.with_type_parameter_rib(HasTypeParameters(generics,
Expand All @@ -2077,40 +2086,42 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
this.visit_generics(generics);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it possibly make sense to first do the this.with_current_self_type(self_type, |this| { ... }) and only do the this.visit_generics(generics); within the { ... }? It seems like it might fix #24944

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(alternatively, maybe we should first visit the generics.ty_params outside of the with_current_self_type, and then just move the visit of the generics.where_clause.predicates into the inside of with_current_self_type.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(well, my attempt to quickly hack this in was met with failure: internal compiler error: self type has not been fully resolved)


// Resolve the trait reference, if necessary.
this.with_optional_trait_ref(opt_trait_reference.as_ref(), |this| {
this.with_optional_trait_ref(opt_trait_reference.as_ref(), |this, trait_id| {
// Resolve the self type.
this.visit_ty(self_type);

this.with_current_self_type(self_type, |this| {
for impl_item in impl_items {
match impl_item.node {
MethodImplItem(ref sig, _) => {
// If this is a trait impl, ensure the method
// exists in trait
this.check_trait_item(impl_item.ident.name,
impl_item.span);

// We also need a new scope for the method-
// specific type parameters.
let type_parameters =
HasTypeParameters(&sig.generics,
FnSpace,
MethodRibKind);
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_impl_item(this, impl_item);
});
}
TypeImplItem(ref ty) => {
// If this is a trait impl, ensure the method
// exists in trait
this.check_trait_item(impl_item.ident.name,
impl_item.span);
this.with_self_rib(DefSelfTy(trait_id, Some((item_id, self_type.id))), |this| {
this.with_current_self_type(self_type, |this| {
for impl_item in impl_items {
match impl_item.node {
MethodImplItem(ref sig, _) => {
// If this is a trait impl, ensure the method
// exists in trait
this.check_trait_item(impl_item.ident.name,
impl_item.span);

// We also need a new scope for the method-
// specific type parameters.
let type_parameters =
HasTypeParameters(&sig.generics,
FnSpace,
MethodRibKind);
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_impl_item(this, impl_item);
});
}
TypeImplItem(ref ty) => {
// If this is a trait impl, ensure the method
// exists in trait
this.check_trait_item(impl_item.ident.name,
impl_item.span);

this.visit_ty(ty);
this.visit_ty(ty);
}
ast::MacImplItem(_) => {}
}
ast::MacImplItem(_) => {}
}
}
});
});
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/save/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ impl <'l, 'tcx> DxrVisitor<'l, 'tcx> {

def::DefFn(..) => Some(recorder::FnRef),

def::DefSelfTy(_) |
def::DefSelfTy(..) |
def::DefRegion(_) |
def::DefLabel(_) |
def::DefTyParam(..) |
Expand Down
Loading