Skip to content

Commit

Permalink
Auto merge of #31857 - jseyfried:fix_scoping, r=nikomatsakis
Browse files Browse the repository at this point in the history
This fixes a bug (#31845) introduced in #31105 in which lexical scopes contain items from all anonymous module ancestors, even if the path to the anonymous module includes a normal module:
```rust
fn f() {
    fn g() {}
    mod foo {
        fn h() {
           g(); // This erroneously resolves on nightly
        }
    }
}
```

This is a [breaking-change] on nightly but not on stable or beta.
r? @nikomatsakis
  • Loading branch information
bors committed Feb 26, 2016
2 parents 15e9a95 + 2942cf7 commit 9c6a008
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 19 deletions.
46 changes: 27 additions & 19 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,8 @@ enum RibKind<'a> {
// We're in a constant item. Can't refer to dynamic stuff.
ConstantItemRibKind,

// We passed through an anonymous module.
AnonymousModuleRibKind(Module<'a>),
// We passed through a module.
ModuleRibKind(Module<'a>),
}

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -1680,16 +1680,20 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn with_scope<F>(&mut self, id: NodeId, f: F)
where F: FnOnce(&mut Resolver)
{
let orig_module = self.current_module;
if let Some(module) = self.current_module.module_children.borrow().get(&id) {
// Move down in the graph.
let orig_module = ::std::mem::replace(&mut self.current_module, module);
self.value_ribs.push(Rib::new(ModuleRibKind(module)));
self.type_ribs.push(Rib::new(ModuleRibKind(module)));

// Move down in the graph.
if let Some(module) = orig_module.module_children.borrow().get(&id) {
self.current_module = module;
}
f(self);

f(self);

self.current_module = orig_module;
self.current_module = orig_module;
self.value_ribs.pop();
self.type_ribs.pop();
} else {
f(self);
}
}

/// Searches the current set of local scopes for labels.
Expand Down Expand Up @@ -2266,8 +2270,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

if let Some(anonymous_module) = anonymous_module {
debug!("(resolving block) found anonymous module, moving down");
self.value_ribs.push(Rib::new(AnonymousModuleRibKind(anonymous_module)));
self.type_ribs.push(Rib::new(AnonymousModuleRibKind(anonymous_module)));
self.value_ribs.push(Rib::new(ModuleRibKind(anonymous_module)));
self.type_ribs.push(Rib::new(ModuleRibKind(anonymous_module)));
self.current_module = anonymous_module;
} else {
self.value_ribs.push(Rib::new(NormalRibKind));
Expand Down Expand Up @@ -2811,8 +2815,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

if check_ribs {
if let Some(def) = self.resolve_identifier_in_local_ribs(identifier, namespace) {
return Some(def);
match self.resolve_identifier_in_local_ribs(identifier, namespace, record_used) {
Some(def) => return Some(def),
None => {}
}
}

Expand Down Expand Up @@ -2844,7 +2849,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Def::Local(_, node_id) => {
for rib in ribs {
match rib.kind {
NormalRibKind | AnonymousModuleRibKind(..) => {
NormalRibKind | ModuleRibKind(..) => {
// Nothing to do. Continue.
}
ClosureRibKind(function_id) => {
Expand Down Expand Up @@ -2893,7 +2898,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
for rib in ribs {
match rib.kind {
NormalRibKind | MethodRibKind | ClosureRibKind(..) |
AnonymousModuleRibKind(..) => {
ModuleRibKind(..) => {
// Nothing to do. Continue.
}
ItemRibKind => {
Expand Down Expand Up @@ -3024,7 +3029,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

fn resolve_identifier_in_local_ribs(&mut self,
ident: hir::Ident,
namespace: Namespace)
namespace: Namespace,
record_used: bool)
-> Option<LocalDef> {
// Check the local set of ribs.
let name = match namespace { ValueNS => ident.name, TypeNS => ident.unhygienic_name };
Expand All @@ -3051,16 +3057,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

if let AnonymousModuleRibKind(module) = self.get_ribs(namespace)[i].kind {
if let ModuleRibKind(module) = self.get_ribs(namespace)[i].kind {
if let Success(binding) = self.resolve_name_in_module(module,
ident.unhygienic_name,
namespace,
true,
true) {
record_used) {
if let Some(def) = binding.def() {
return Some(LocalDef::from_def(def));
}
}
// We can only see through anonymous modules
if module.def.is_some() { return None; }
}
}

Expand Down
22 changes: 22 additions & 0 deletions src/test/compile-fail/issue-31845.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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.

// Checks lexical scopes cannot see through normal module boundries

fn f() {
fn g() {}
mod foo {
fn h() {
g(); //~ ERROR unresolved name
}
}
}

fn main() {}
7 changes: 7 additions & 0 deletions src/test/compile-fail/lint-unused-imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ mod bar {
}
}

fn g() {
use self::g; //~ ERROR unused import
fn f() {
self::g();
}
}

fn main() {
cal(foo::Point{x:3, y:9});
let mut a = 3;
Expand Down

0 comments on commit 9c6a008

Please sign in to comment.