From faf0852fc1d01aef18fe8098a0f2f601dbfebd9b Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Fri, 22 Jan 2016 09:55:29 +0000 Subject: [PATCH] Resolve: fix #23880, a scoping bug This fixes a bug in which items in a block are shadowed by local variables and type parameters that are in scope. It is a [breaking-change]. For example, the following code breaks: ```rust fn foo() { let mut f = 1; { fn f() {} f += 1; // This will now resolve to the function instead of the local variable } } ``` Any breakage can be fixed by renaming the item that is no longer shadowed. --- src/librustc_resolve/lib.rs | 80 +++++++++++++++++----------- src/test/run-pass/lexical-scoping.rs | 28 ++++++++++ 2 files changed, 78 insertions(+), 30 deletions(-) create mode 100644 src/test/run-pass/lexical-scoping.rs diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 444c43163e3..ee1150933e1 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -723,7 +723,7 @@ enum FallbackSuggestion { } #[derive(Copy, Clone)] -enum TypeParameters<'a> { +enum TypeParameters<'tcx, 'a> { NoTypeParameters, HasTypeParameters(// Type parameters. &'a Generics, @@ -733,13 +733,13 @@ enum TypeParameters<'a> { ParamSpace, // The kind of the rib used for type parameters. - RibKind), + RibKind<'tcx>), } // The rib kind controls the translation of local // definitions (`Def::Local`) to upvars (`Def::Upvar`). #[derive(Copy, Clone, Debug)] -enum RibKind { +enum RibKind<'a> { // No translation needs to be applied. NormalRibKind, @@ -758,6 +758,9 @@ enum RibKind { // We're in a constant item. Can't refer to dynamic stuff. ConstantItemRibKind, + + // We passed through an anonymous module. + AnonymousModuleRibKind(Module<'a>), } #[derive(Copy, Clone)] @@ -799,13 +802,13 @@ enum BareIdentifierPatternResolution { /// One local scope. #[derive(Debug)] -struct Rib { +struct Rib<'a> { bindings: HashMap, - kind: RibKind, + kind: RibKind<'a>, } -impl Rib { - fn new(kind: RibKind) -> Rib { +impl<'a> Rib<'a> { + fn new(kind: RibKind<'a>) -> Rib<'a> { Rib { bindings: HashMap::new(), kind: kind, @@ -1180,13 +1183,13 @@ pub struct Resolver<'a, 'tcx: 'a> { // The current set of local scopes, for values. // FIXME #4948: Reuse ribs to avoid allocation. - value_ribs: Vec, + value_ribs: Vec>, // The current set of local scopes, for types. - type_ribs: Vec, + type_ribs: Vec>, // The current set of local scopes, for labels. - label_ribs: Vec, + label_ribs: Vec>, // The trait that the current context can refer to. current_trait_ref: Option<(DefId, TraitRef)>, @@ -1304,6 +1307,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.arenas.modules.alloc(ModuleS::new(parent_link, def, external, is_public)) } + fn get_ribs<'b>(&'b mut self, ns: Namespace) -> &'b mut Vec> { + match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs } + } + #[inline] fn record_import_use(&mut self, import_id: NodeId, name: Name) { if !self.make_glob_map { @@ -2122,7 +2129,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - fn with_type_parameter_rib(&mut self, type_parameters: TypeParameters, f: F) + fn with_type_parameter_rib<'b, F>(&'b mut self, type_parameters: TypeParameters<'a, 'b>, f: F) where F: FnOnce(&mut Resolver) { match type_parameters { @@ -2191,7 +2198,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } - fn resolve_function(&mut self, rib_kind: RibKind, declaration: &FnDecl, block: &Block) { + fn resolve_function(&mut self, rib_kind: RibKind<'a>, declaration: &FnDecl, block: &Block) { // Create a value rib for the function. self.value_ribs.push(Rib::new(rib_kind)); @@ -2494,18 +2501,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { fn resolve_block(&mut self, block: &Block) { debug!("(resolving block) entering block"); - self.value_ribs.push(Rib::new(NormalRibKind)); - // Move down in the graph, if there's an anonymous module rooted here. let orig_module = self.current_module; - match orig_module.anonymous_children.borrow().get(&block.id) { - None => { - // Nothing to do. - } - Some(anonymous_module) => { - debug!("(resolving block) found anonymous module, moving down"); - self.current_module = anonymous_module; - } + let anonymous_module = + orig_module.anonymous_children.borrow().get(&block.id).map(|module| *module); + + 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.current_module = anonymous_module; + } else { + self.value_ribs.push(Rib::new(NormalRibKind)); } // Check for imports appearing after non-item statements. @@ -2538,6 +2545,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { if !self.resolved { self.current_module = orig_module; self.value_ribs.pop(); + if let Some(_) = anonymous_module { + self.type_ribs.pop(); + } } debug!("(resolving block) leaving block"); } @@ -3072,7 +3082,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Def::Local(_, node_id) => { for rib in ribs { match rib.kind { - NormalRibKind => { + NormalRibKind | AnonymousModuleRibKind(..) => { // Nothing to do. Continue. } ClosureRibKind(function_id) => { @@ -3120,7 +3130,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Def::TyParam(..) | Def::SelfTy(..) => { for rib in ribs { match rib.kind { - NormalRibKind | MethodRibKind | ClosureRibKind(..) => { + NormalRibKind | MethodRibKind | ClosureRibKind(..) | + AnonymousModuleRibKind(..) => { // Nothing to do. Continue. } ItemRibKind => { @@ -3271,13 +3282,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { namespace: Namespace) -> Option { // Check the local set of ribs. - let (name, ribs) = match namespace { - ValueNS => (ident.name, &self.value_ribs), - TypeNS => (ident.unhygienic_name, &self.type_ribs), - }; + let name = match namespace { ValueNS => ident.name, TypeNS => ident.unhygienic_name }; - for (i, rib) in ribs.iter().enumerate().rev() { - if let Some(def_like) = rib.bindings.get(&name).cloned() { + for i in (0 .. self.get_ribs(namespace).len()).rev() { + if let Some(def_like) = self.get_ribs(namespace)[i].bindings.get(&name).cloned() { match def_like { DlDef(def) => { debug!("(resolving path in local ribs) resolved `{}` to {:?} at {}", @@ -3297,6 +3305,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } } + + if let AnonymousModuleRibKind(module) = self.get_ribs(namespace)[i].kind { + if let Success((target, _)) = self.resolve_name_in_module(module, + ident.unhygienic_name, + namespace, + PathSearch, + true) { + if let Some(def) = target.binding.def() { + return Some(LocalDef::from_def(def)); + } + } + } } None diff --git a/src/test/run-pass/lexical-scoping.rs b/src/test/run-pass/lexical-scoping.rs new file mode 100644 index 00000000000..36604042d0f --- /dev/null +++ b/src/test/run-pass/lexical-scoping.rs @@ -0,0 +1,28 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that items in subscopes can shadow type parameters and local variables (see issue #23880). + +#![allow(unused)] +struct Foo { x: Box } +impl Foo { + fn foo(&self) { + type Bar = i32; + let _: Bar = 42; + } +} + +fn main() { + let f = 1; + { + fn f() {} + f(); + } +}