OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #32141 - jseyfried:fix_resolution_in_lexical_scopes, r=nikomatsakis

Browse Source 
Fix name resolution in lexical scopes

Currently, `resolve_item_in_lexical_scope` does not check the "ribs" (type parameters and local variables). This can allow items that should be shadowed by type parameters to be named.

For example,
```rust
struct T { i: i32 }
fn f<T>() {
    let t = T { i: 0 }; // This use of `T` resolves to the struct, not the type parameter
}

mod Foo {
    pub fn f() {}
}
fn g<Foo>() {
    Foo::f(); // This use of `Foo` resolves to the module, not the type parameter
}
```

This PR changes `resolve_item_in_lexical_scope` so that it fails when the item is shadowed by a rib (fixes #32120).
This is a [breaking-change], but it looks unlikely to cause breakage in practice.

r? @nikomatsakis
This commit is contained in:
bors 2016-03-12 21:55:14 -08:00
commit 06074ac004
3 changed files with 75 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
src/librustc_resolve/lib.rs
View File

@ -1166,8 +1166,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
unresolved_imports: 0, unresolved_imports: 0,
current_module: graph_root, current_module: graph_root,
value_ribs: Vec::new(), value_ribs: vec![Rib::new(ModuleRibKind(graph_root))],
type_ribs: Vec::new(), type_ribs: vec![Rib::new(ModuleRibKind(graph_root))],
label_ribs: Vec::new(), label_ribs: Vec::new(),
current_trait_ref: None, current_trait_ref: None,
@ -1354,7 +1354,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
/// On success, returns the resolved module, and the closest *private* /// On success, returns the resolved module, and the closest *private*
/// module found to the destination when resolving this path. /// module found to the destination when resolving this path.
fn resolve_module_path(&mut self, fn resolve_module_path(&mut self,
module_: Module<'a>,
module_path: &[Name], module_path: &[Name],
use_lexical_scope: UseLexicalScopeFlag, use_lexical_scope: UseLexicalScopeFlag,
span: Span) span: Span)
@ -1365,10 +1364,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
debug!("(resolving module path for import) processing `{}` rooted at `{}`", debug!("(resolving module path for import) processing `{}` rooted at `{}`",
names_to_string(module_path), names_to_string(module_path),
module_to_string(&module_)); module_to_string(self.current_module));
// Resolve the module prefix, if any. // Resolve the module prefix, if any.
let module_prefix_result = self.resolve_module_prefix(module_, module_path); let module_prefix_result = self.resolve_module_prefix(module_path);
let search_module; let search_module;
let start_index; let start_index;
@ -1410,8 +1409,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// This is not a crate-relative path. We resolve the // This is not a crate-relative path. We resolve the
// first component of the path in the current lexical // first component of the path in the current lexical
// scope and then proceed to resolve below that. // scope and then proceed to resolve below that.
match self.resolve_item_in_lexical_scope(module_, match self.resolve_item_in_lexical_scope(module_path[0],
module_path[0],
TypeNS, TypeNS,
true) { true) {
Failed(err) => return Failed(err), Failed(err) => return Failed(err),
@ -1442,64 +1440,40 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
span) span)
} }
/// This function resolves `name` in `namespace` in the current lexical scope, returning
/// Success(binding) if `name` resolves to an item, or Failed(None) if `name` does not resolve
/// or resolves to a type parameter or local variable.
/// n.b. `resolve_identifier_in_local_ribs` also resolves names in the current lexical scope.
///
/// Invariant: This must only be called during main resolution, not during /// Invariant: This must only be called during main resolution, not during
/// import resolution. /// import resolution.
fn resolve_item_in_lexical_scope(&mut self, fn resolve_item_in_lexical_scope(&mut self,
module_: Module<'a>,
name: Name, name: Name,
namespace: Namespace, namespace: Namespace,
record_used: bool) record_used: bool)
-> ResolveResult<&'a NameBinding<'a>> { -> ResolveResult<&'a NameBinding<'a>> {
debug!("(resolving item in lexical scope) resolving `{}` in namespace {:?} in `{}`", // Walk backwards up the ribs in scope.
name, for i in (0 .. self.get_ribs(namespace).len()).rev() {
namespace, if let Some(_) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() {
module_to_string(&module_)); // The name resolves to a type parameter or local variable, so return Failed(None).
return Failed(None);
}
// Proceed up the scope chain looking for parent modules. if let ModuleRibKind(module) = self.get_ribs(namespace)[i].kind {
let mut search_module = module_; if let Success(binding) = self.resolve_name_in_module(module,
loop { name,
// Resolve the name in the parent module. namespace,
match self.resolve_name_in_module(search_module, name, namespace, true, record_used) { true,
Failed(Some((span, msg))) => { record_used) {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg)); // The name resolves to an item.
}
Failed(None) => (), // Continue up the search chain.
Indeterminate => {
// We couldn't see through the higher scope because of an
// unresolved import higher up. Bail.
debug!("(resolving item in lexical scope) indeterminate higher scope; bailing");
return Indeterminate;
}
Success(binding) => {
// We found the module.
debug!("(resolving item in lexical scope) found name in module, done");
return Success(binding); return Success(binding);
} }
} // We can only see through anonymous modules
if module.def.is_some() { return Failed(None); }
// Go to the next parent.
match search_module.parent_link {
NoParentLink => {
// No more parents. This module was unresolved.
debug!("(resolving item in lexical scope) unresolved module: no parent module");
return Failed(None);
}
ModuleParentLink(parent_module_node, _) => {
if search_module.is_normal() {
// We stop the search here.
debug!("(resolving item in lexical scope) unresolved module: not \
searching through module parents");
return Failed(None);
} else {
search_module = parent_module_node;
}
}
BlockParentLink(parent_module_node, _) => {
search_module = parent_module_node;
}
} }
} }
Failed(None)
} }
/// Returns the nearest normal module parent of the given module. /// Returns the nearest normal module parent of the given module.
@ -1535,9 +1509,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
/// Resolves a "module prefix". A module prefix is one or both of (a) `self::`; /// Resolves a "module prefix". A module prefix is one or both of (a) `self::`;
/// (b) some chain of `super::`. /// (b) some chain of `super::`.
/// grammar: (SELF MOD_SEP ) ? (SUPER MOD_SEP) * /// grammar: (SELF MOD_SEP ) ? (SUPER MOD_SEP) *
fn resolve_module_prefix(&mut self, fn resolve_module_prefix(&mut self, module_path: &[Name])
module_: Module<'a>,
module_path: &[Name])
-> ResolveResult<ModulePrefixResult<'a>> { -> ResolveResult<ModulePrefixResult<'a>> {
// Start at the current module if we see `self` or `super`, or at the // Start at the current module if we see `self` or `super`, or at the
// top of the crate otherwise. // top of the crate otherwise.
@ -1546,6 +1518,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
"super" => 0, "super" => 0,
_ => return Success(NoPrefixFound), _ => return Success(NoPrefixFound),
}; };
let module_ = self.current_module;
let mut containing_module = self.get_nearest_normal_module_parent_or_self(module_); let mut containing_module = self.get_nearest_normal_module_parent_or_self(module_);
// Now loop through all the `super`s we find. // Now loop through all the `super`s we find.
@ -1905,7 +1878,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
trait_path: &Path, trait_path: &Path,
path_depth: usize) path_depth: usize)
-> Result<PathResolution, ()> { -> Result<PathResolution, ()> {
if let Some(path_res) = self.resolve_path(id, trait_path, path_depth, TypeNS, true) { if let Some(path_res) = self.resolve_path(id, trait_path, path_depth, TypeNS) {
if let Def::Trait(_) = path_res.base_def { if let Def::Trait(_) = path_res.base_def {
debug!("(resolving trait) found trait def: {:?}", path_res); debug!("(resolving trait) found trait def: {:?}", path_res);
Ok(path_res) Ok(path_res)
@ -1963,7 +1936,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&hir::WherePredicate::BoundPredicate(_) | &hir::WherePredicate::BoundPredicate(_) |
&hir::WherePredicate::RegionPredicate(_) => {} &hir::WherePredicate::RegionPredicate(_) => {}
&hir::WherePredicate::EqPredicate(ref eq_pred) => { &hir::WherePredicate::EqPredicate(ref eq_pred) => {
let path_res = self.resolve_path(eq_pred.id, &eq_pred.path, 0, TypeNS, true); let path_res = self.resolve_path(eq_pred.id, &eq_pred.path, 0, TypeNS);
if let Some(PathResolution { base_def: Def::TyParam(..), .. }) = path_res { if let Some(PathResolution { base_def: Def::TyParam(..), .. }) = path_res {
self.record_def(eq_pred.id, path_res.unwrap()); self.record_def(eq_pred.id, path_res.unwrap());
} else { } else {
@ -2229,8 +2202,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let resolution = match self.resolve_possibly_assoc_item(ty.id, let resolution = match self.resolve_possibly_assoc_item(ty.id,
maybe_qself.as_ref(), maybe_qself.as_ref(),
path, path,
TypeNS, TypeNS) {
true) {
// `<T>::a::b::c` is resolved by typeck alone. // `<T>::a::b::c` is resolved by typeck alone.
TypecheckRequired => { TypecheckRequired => {
// Resolve embedded types. // Resolve embedded types.
@ -2255,7 +2227,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.record_def(ty.id, err_path_resolution()); self.record_def(ty.id, err_path_resolution());
// Keep reporting some errors even if they're ignored above. // Keep reporting some errors even if they're ignored above.
self.resolve_path(ty.id, path, 0, TypeNS, true); self.resolve_path(ty.id, path, 0, TypeNS);
let kind = if maybe_qself.is_some() { let kind = if maybe_qself.is_some() {
"associated type" "associated type"
@ -2433,8 +2405,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let resolution = match self.resolve_possibly_assoc_item(pat_id, let resolution = match self.resolve_possibly_assoc_item(pat_id,
None, None,
path, path,
ValueNS, ValueNS) {
false) {
// The below shouldn't happen because all // The below shouldn't happen because all
// qualified paths should be in PatKind::QPath. // qualified paths should be in PatKind::QPath.
TypecheckRequired => TypecheckRequired =>
@ -2506,8 +2477,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let resolution = match self.resolve_possibly_assoc_item(pat_id, let resolution = match self.resolve_possibly_assoc_item(pat_id,
Some(qself), Some(qself),
path, path,
ValueNS, ValueNS) {
false) {
TypecheckRequired => { TypecheckRequired => {
// All `<T>::CONST` should end up here, and will // All `<T>::CONST` should end up here, and will
// require use of the trait map to resolve // require use of the trait map to resolve
@ -2557,7 +2527,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} }
PatKind::Struct(ref path, _, _) => { PatKind::Struct(ref path, _, _) => {
match self.resolve_path(pat_id, path, 0, TypeNS, false) { match self.resolve_path(pat_id, path, 0, TypeNS) {
Some(definition) => { Some(definition) => {
self.record_def(pattern.id, definition); self.record_def(pattern.id, definition);
} }
@ -2591,8 +2561,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
name: Name, name: Name,
span: Span) span: Span)
-> BareIdentifierPatternResolution { -> BareIdentifierPatternResolution {
let module = self.current_module; match self.resolve_item_in_lexical_scope(name, ValueNS, true) {
match self.resolve_item_in_lexical_scope(module, name, ValueNS, true) {
Success(binding) => { Success(binding) => {
debug!("(resolve bare identifier pattern) succeeded in finding {} at {:?}", debug!("(resolve bare identifier pattern) succeeded in finding {} at {:?}",
name, name,
@ -2639,8 +2608,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
id: NodeId, id: NodeId,
maybe_qself: Option<&hir::QSelf>, maybe_qself: Option<&hir::QSelf>,
path: &Path, path: &Path,
namespace: Namespace, namespace: Namespace)
check_ribs: bool)
-> AssocItemResolveResult { -> AssocItemResolveResult {
let max_assoc_types; let max_assoc_types;
@ -2659,14 +2627,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} }
let mut resolution = self.with_no_errors(|this| { let mut resolution = self.with_no_errors(|this| {
this.resolve_path(id, path, 0, namespace, check_ribs) this.resolve_path(id, path, 0, namespace)
}); });
for depth in 1..max_assoc_types { for depth in 1..max_assoc_types {
if resolution.is_some() { if resolution.is_some() {
break; break;
} }
self.with_no_errors(|this| { self.with_no_errors(|this| {
resolution = this.resolve_path(id, path, depth, TypeNS, true); resolution = this.resolve_path(id, path, depth, TypeNS);
}); });
} }
if let Some(Def::Mod(_)) = resolution.map(|r| r.base_def) { if let Some(Def::Mod(_)) = resolution.map(|r| r.base_def) {
@ -2676,16 +2644,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ResolveAttempt(resolution) ResolveAttempt(resolution)
} }
/// If `check_ribs` is true, checks the local definitions first; i.e.
/// doesn't skip straight to the containing module.
/// Skips `path_depth` trailing segments, which is also reflected in the /// Skips `path_depth` trailing segments, which is also reflected in the
/// returned value. See `middle::def::PathResolution` for more info. /// returned value. See `middle::def::PathResolution` for more info.
pub fn resolve_path(&mut self, pub fn resolve_path(&mut self,
id: NodeId, id: NodeId,
path: &Path, path: &Path,
path_depth: usize, path_depth: usize,
namespace: Namespace, namespace: Namespace)
check_ribs: bool)
-> Option<PathResolution> { -> Option<PathResolution> {
let span = path.span; let span = path.span;
let segments = &path.segments[..path.segments.len() - path_depth]; let segments = &path.segments[..path.segments.len() - path_depth];
@ -2700,14 +2665,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Try to find a path to an item in a module. // Try to find a path to an item in a module.
let last_ident = segments.last().unwrap().identifier; let last_ident = segments.last().unwrap().identifier;
if segments.len() <= 1 { if segments.len() <= 1 {
let unqualified_def = self.resolve_identifier(last_ident, namespace, check_ribs, true); let unqualified_def = self.resolve_identifier(last_ident, namespace, true);
return unqualified_def.and_then(|def| self.adjust_local_def(def, span)) return unqualified_def.and_then(|def| self.adjust_local_def(def, span))
.map(|def| { .map(|def| {
PathResolution::new(def, path_depth) PathResolution::new(def, path_depth)
}); });
} }
let unqualified_def = self.resolve_identifier(last_ident, namespace, check_ribs, false); let unqualified_def = self.resolve_identifier(last_ident, namespace, false);
let def = self.resolve_module_relative_path(span, segments, namespace); let def = self.resolve_module_relative_path(span, segments, namespace);
match (def, unqualified_def) { match (def, unqualified_def) {
(Some(d), Some(ref ud)) if d == ud.def => { (Some(d), Some(ref ud)) if d == ud.def => {
@ -2727,7 +2692,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn resolve_identifier(&mut self, fn resolve_identifier(&mut self,
identifier: hir::Ident, identifier: hir::Ident,
namespace: Namespace, namespace: Namespace,
check_ribs: bool,
record_used: bool) record_used: bool)
-> Option<LocalDef> { -> Option<LocalDef> {
if identifier.name == special_idents::invalid.name { if identifier.name == special_idents::invalid.name {
@ -2743,24 +2707,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} }
} }
if check_ribs { self.resolve_identifier_in_local_ribs(identifier, namespace, record_used)
match self.resolve_identifier_in_local_ribs(identifier, namespace, record_used) {
Some(def) => return Some(def),
None => {}
}
}
// Check the items.
let module = self.current_module;
let name = identifier.unhygienic_name;
match self.resolve_item_in_lexical_scope(module, name, namespace, record_used) {
Success(binding) => binding.def().map(LocalDef::from_def),
Failed(Some((span, msg))) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
None
}
_ => None,
}
} }
// Resolve a local definition, potentially adjusting for closures. // Resolve a local definition, potentially adjusting for closures.
@ -2866,8 +2813,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let containing_module; let containing_module;
let current_module = self.current_module; match self.resolve_module_path(&module_path, UseLexicalScope, span) {
match self.resolve_module_path(current_module, &module_path, UseLexicalScope, span) {
Failed(err) => { Failed(err) => {
let (span, msg) = match err { let (span, msg) = match err {
Some((span, msg)) => (span, msg), Some((span, msg)) => (span, msg),
@ -3021,7 +2967,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
span: Span, span: Span,
name_path: &[ast::Name]) name_path: &[ast::Name])
-> Option<Module<'a>> { -> Option<Module<'a>> {
let root = this.current_module;
let last_name = name_path.last().unwrap(); let last_name = name_path.last().unwrap();
if name_path.len() == 1 { if name_path.len() == 1 {
@ -3031,7 +2976,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
.and_then(NameBinding::module) .and_then(NameBinding::module)
} }
} else { } else {
this.resolve_module_path(root, &name_path, UseLexicalScope, span).success() this.resolve_module_path(&name_path, UseLexicalScope, span).success()
} }
} }
@ -3142,8 +3087,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let resolution = match self.resolve_possibly_assoc_item(expr.id, let resolution = match self.resolve_possibly_assoc_item(expr.id,
maybe_qself.as_ref(), maybe_qself.as_ref(),
path, path,
ValueNS, ValueNS) {
true) {
// `<T>::a::b::c` is resolved by typeck alone. // `<T>::a::b::c` is resolved by typeck alone.
TypecheckRequired => { TypecheckRequired => {
let method_name = path.segments.last().unwrap().identifier.name; let method_name = path.segments.last().unwrap().identifier.name;
@ -3203,7 +3147,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// structs, which wouldn't result in this error.) // structs, which wouldn't result in this error.)
let path_name = path_names_to_string(path, 0); let path_name = path_names_to_string(path, 0);
let type_res = self.with_no_errors(|this| { let type_res = self.with_no_errors(|this| {
this.resolve_path(expr.id, path, 0, TypeNS, false) this.resolve_path(expr.id, path, 0, TypeNS)
}); });
self.record_def(expr.id, err_path_resolution()); self.record_def(expr.id, err_path_resolution());
@ -3224,7 +3168,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
} }
_ => { _ => {
// Keep reporting some errors even if they're ignored above. // Keep reporting some errors even if they're ignored above.
self.resolve_path(expr.id, path, 0, ValueNS, true); self.resolve_path(expr.id, path, 0, ValueNS);
let mut method_scope = false; let mut method_scope = false;
self.value_ribs.iter().rev().all(|rib| { self.value_ribs.iter().rev().all(|rib| {
@ -3271,10 +3215,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let name_path = path.segments.iter() let name_path = path.segments.iter()
.map(|seg| seg.identifier.name) .map(|seg| seg.identifier.name)
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let current_module = self.current_module;
match self.resolve_module_path(current_module, match self.resolve_module_path(&name_path[..],
&name_path[..],
UseLexicalScope, UseLexicalScope,
expr.span) { expr.span) {
Success(_) => { Success(_) => {
@ -3300,7 +3242,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Resolve the path to the structure it goes to. We don't // Resolve the path to the structure it goes to. We don't
// check to ensure that the path is actually a structure; that // check to ensure that the path is actually a structure; that
// is checked later during typeck. // is checked later during typeck.
match self.resolve_path(expr.id, path, 0, TypeNS, false) { match self.resolve_path(expr.id, path, 0, TypeNS) {
Some(definition) => self.record_def(expr.id, definition), Some(definition) => self.record_def(expr.id, definition),
None => { None => {
debug!("(resolving expression) didn't find struct def",); debug!("(resolving expression) didn't find struct def",);

3
src/librustc_resolve/resolve_imports.rs
View File

@ -444,8 +444,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
module_to_string(&module_)); module_to_string(&module_));
self.resolver self.resolver
.resolve_module_path(module_, .resolve_module_path(&import_directive.module_path,
&import_directive.module_path,
UseLexicalScopeFlag::DontUseLexicalScope, UseLexicalScopeFlag::DontUseLexicalScope,
import_directive.span) import_directive.span)
.and_then(|containing_module| { .and_then(|containing_module| {

23
src/test/compile-fail/lexical-scopes.rs Normal file
View File

@ -0,0 +1,23 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
struct T { i: i32 }
fn f<T>() {
let t = T { i: 0 }; //~ ERROR `T` does not name a structure
}
mod Foo {
pub fn f() {}
}
fn g<Foo>() {
Foo::f(); //~ ERROR no associated item named `f`
}
fn main() {}