From 428ef191e0ff3a26bb571efcdc6d7221c2ded07b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 30 Apr 2018 02:20:14 +0300 Subject: [PATCH 1/2] resolve (cleanup): Get rid of `Option` in `PerNS` --- src/librustc_resolve/build_reduced_graph.rs | 8 ++++++-- src/librustc_resolve/lib.rs | 21 +++++++++------------ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 14ceb5f59a3..f4a5ace69dd 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -17,7 +17,7 @@ use macros::{InvocationData, LegacyScope}; use resolve_imports::ImportDirective; use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport}; use {Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, ToNameBinding}; -use {Resolver, ResolverArenas}; +use {PerNS, Resolver, ResolverArenas}; use Namespace::{self, TypeNS, ValueNS, MacroNS}; use {resolve_error, resolve_struct_error, ResolutionError}; @@ -175,7 +175,11 @@ impl<'a> Resolver<'a> { let subclass = SingleImport { target: ident, source, - result: self.per_ns(|_, _| Cell::new(Err(Undetermined))), + result: PerNS { + type_ns: Cell::new(Err(Undetermined)), + value_ns: Cell::new(Err(Undetermined)), + macro_ns: Cell::new(Err(Undetermined)), + }, type_ns_only, }; self.add_import_directive( diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 127331152c1..cb8c38b92b5 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -700,7 +700,7 @@ pub enum Namespace { pub struct PerNS { value_ns: T, type_ns: T, - macro_ns: Option, + macro_ns: T, } impl ::std::ops::Index for PerNS { @@ -709,7 +709,7 @@ impl ::std::ops::Index for PerNS { match ns { ValueNS => &self.value_ns, TypeNS => &self.type_ns, - MacroNS => self.macro_ns.as_ref().unwrap(), + MacroNS => &self.macro_ns, } } } @@ -719,7 +719,7 @@ impl ::std::ops::IndexMut for PerNS { match ns { ValueNS => &mut self.value_ns, TypeNS => &mut self.type_ns, - MacroNS => self.macro_ns.as_mut().unwrap(), + MacroNS => &mut self.macro_ns, } } } @@ -1726,7 +1726,7 @@ impl<'a> Resolver<'a> { ribs: PerNS { value_ns: vec![Rib::new(ModuleRibKind(graph_root))], type_ns: vec![Rib::new(ModuleRibKind(graph_root))], - macro_ns: Some(vec![Rib::new(ModuleRibKind(graph_root))]), + macro_ns: vec![Rib::new(ModuleRibKind(graph_root))], }, label_ribs: Vec::new(), @@ -1806,14 +1806,11 @@ impl<'a> Resolver<'a> { } /// Runs the function on each namespace. - fn per_ns T>(&mut self, mut f: F) -> PerNS { - PerNS { - type_ns: f(self, TypeNS), - value_ns: f(self, ValueNS), - macro_ns: match self.use_extern_macros { - true => Some(f(self, MacroNS)), - false => None, - }, + fn per_ns(&mut self, mut f: F) { + f(self, TypeNS); + f(self, ValueNS); + if self.use_extern_macros { + f(self, MacroNS); } } From d5e31158a204aaa8ddc46bd8b9f44a4770db5c4d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 1 May 2018 03:12:36 +0300 Subject: [PATCH 2/2] Better support for import resolution in 3 namespaces --- src/librustc_resolve/resolve_imports.rs | 80 ++++++++++++++++++------- src/test/ui/issue-50187.rs | 49 +++++++++++++++ 2 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/issue-50187.rs diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index e2a7f5668d2..17aa510b565 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -33,7 +33,7 @@ use syntax::util::lev_distance::find_best_match_for_name; use syntax_pos::Span; use std::cell::{Cell, RefCell}; -use std::mem; +use std::{mem, ptr}; /// Contains data for specific types of import directives. #[derive(Clone, Debug)] @@ -89,6 +89,8 @@ enum SingleImports<'a> { None, /// Only the given single import can define the name in the namespace. MaybeOne(&'a ImportDirective<'a>), + /// Only one of these two single imports can define the name in the namespace. + MaybeTwo(&'a ImportDirective<'a>, &'a ImportDirective<'a>), /// At least one single import will define the name in the namespace. AtLeastOne, } @@ -101,21 +103,28 @@ impl<'a> Default for SingleImports<'a> { } impl<'a> SingleImports<'a> { - fn add_directive(&mut self, directive: &'a ImportDirective<'a>) { + fn add_directive(&mut self, directive: &'a ImportDirective<'a>, use_extern_macros: bool) { match *self { SingleImports::None => *self = SingleImports::MaybeOne(directive), - // If two single imports can define the name in the namespace, we can assume that at - // least one of them will define it since otherwise both would have to define only one - // namespace, leading to a duplicate error. - SingleImports::MaybeOne(_) => *self = SingleImports::AtLeastOne, + SingleImports::MaybeOne(directive_one) => *self = if use_extern_macros { + SingleImports::MaybeTwo(directive_one, directive) + } else { + SingleImports::AtLeastOne + }, + // If three single imports can define the name in the namespace, we can assume that at + // least one of them will define it since otherwise we'd get duplicate errors in one of + // other namespaces. + SingleImports::MaybeTwo(..) => *self = SingleImports::AtLeastOne, SingleImports::AtLeastOne => {} }; } - fn directive_failed(&mut self) { + fn directive_failed(&mut self, dir: &'a ImportDirective<'a>) { match *self { SingleImports::None => unreachable!(), SingleImports::MaybeOne(_) => *self = SingleImports::None, + SingleImports::MaybeTwo(dir1, dir2) => + *self = SingleImports::MaybeOne(if ptr::eq(dir1, dir) { dir1 } else { dir2 }), SingleImports::AtLeastOne => {} } } @@ -199,23 +208,50 @@ impl<'a> Resolver<'a> { } // Check if a single import can still define the name. + let resolve_single_import = |this: &mut Self, directive: &'a ImportDirective<'a>| { + let module = match directive.imported_module.get() { + Some(module) => module, + None => return false, + }; + let ident = match directive.subclass { + SingleImport { source, .. } => source, + _ => unreachable!(), + }; + match this.resolve_ident_in_module(module, ident, ns, false, false, path_span) { + Err(Determined) => {} + _ => return false, + } + true + }; match resolution.single_imports { SingleImports::AtLeastOne => return Err(Undetermined), - SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis.get()) => { - let module = match directive.imported_module.get() { - Some(module) => module, - None => return Err(Undetermined), - }; - let ident = match directive.subclass { - SingleImport { source, .. } => source, - _ => unreachable!(), - }; - match self.resolve_ident_in_module(module, ident, ns, false, false, path_span) { - Err(Determined) => {} - _ => return Err(Undetermined), + SingleImports::MaybeOne(directive) => { + let accessible = self.is_accessible(directive.vis.get()); + if accessible { + if !resolve_single_import(self, directive) { + return Err(Undetermined) + } } } - SingleImports::MaybeOne(_) | SingleImports::None => {}, + SingleImports::MaybeTwo(directive1, directive2) => { + let accessible1 = self.is_accessible(directive1.vis.get()); + let accessible2 = self.is_accessible(directive2.vis.get()); + if accessible1 && accessible2 { + if !resolve_single_import(self, directive1) && + !resolve_single_import(self, directive2) { + return Err(Undetermined) + } + } else if accessible1 { + if !resolve_single_import(self, directive1) { + return Err(Undetermined) + } + } else { + if !resolve_single_import(self, directive2) { + return Err(Undetermined) + } + } + } + SingleImports::None => {}, } let no_unresolved_invocations = @@ -281,7 +317,7 @@ impl<'a> Resolver<'a> { SingleImport { target, .. } => { self.per_ns(|this, ns| { let mut resolution = this.resolution(current_module, target, ns).borrow_mut(); - resolution.single_imports.add_directive(directive); + resolution.single_imports.add_directive(directive, this.use_extern_macros); }); } // We don't add prelude imports to the globs since they only affect lexical scopes, @@ -575,7 +611,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { Err(Undetermined) => indeterminate = true, Err(Determined) => { this.update_resolution(parent, target, ns, |_, resolution| { - resolution.single_imports.directive_failed() + resolution.single_imports.directive_failed(directive) }); } Ok(binding) if !binding.is_importable() => { diff --git a/src/test/ui/issue-50187.rs b/src/test/ui/issue-50187.rs new file mode 100644 index 00000000000..87acf106393 --- /dev/null +++ b/src/test/ui/issue-50187.rs @@ -0,0 +1,49 @@ +// Copyright 2018 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. + +// compile-pass + +#![feature(use_extern_macros, decl_macro)] + +mod type_ns { + pub type A = u8; +} +mod value_ns { + pub const A: u8 = 0; +} +mod macro_ns { + pub macro A() {} +} + +mod merge2 { + pub use type_ns::A; + pub use value_ns::A; +} +mod merge3 { + pub use type_ns::A; + pub use value_ns::A; + pub use macro_ns::A; +} + +mod use2 { + pub use merge2::A; +} +mod use3 { + pub use merge3::A; +} + +fn main() { + type B2 = use2::A; + let a2 = use2::A; + + type B3 = use3::A; + let a3 = use3::A; + use3::A!(); +}