Skip to content

Fix an unresolved import issue with enabled use_extern_macros #50355

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.

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

Merged
merged 2 commits into from
May 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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(
Expand Down
21 changes: 9 additions & 12 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ pub enum Namespace {
pub struct PerNS<T> {
value_ns: T,
type_ns: T,
macro_ns: Option<T>,
macro_ns: T,
}

impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
Expand All @@ -709,7 +709,7 @@ impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
match ns {
ValueNS => &self.value_ns,
TypeNS => &self.type_ns,
MacroNS => self.macro_ns.as_ref().unwrap(),
MacroNS => &self.macro_ns,
}
}
}
Expand All @@ -719,7 +719,7 @@ impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
match ns {
ValueNS => &mut self.value_ns,
TypeNS => &mut self.type_ns,
MacroNS => self.macro_ns.as_mut().unwrap(),
MacroNS => &mut self.macro_ns,
}
}
}
Expand Down Expand Up @@ -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(),

Expand Down Expand Up @@ -1806,14 +1806,11 @@ impl<'a> Resolver<'a> {
}

/// Runs the function on each namespace.
fn per_ns<T, F: FnMut(&mut Self, Namespace) -> T>(&mut self, mut f: F) -> PerNS<T> {
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<F: FnMut(&mut Self, Namespace)>(&mut self, mut f: F) {
f(self, TypeNS);
f(self, ValueNS);
if self.use_extern_macros {
f(self, MacroNS);
}
}

Expand Down
80 changes: 58 additions & 22 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
}
Expand All @@ -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 => {}
}
}
Expand Down Expand Up @@ -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::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::MaybeOne(_) | SingleImports::None => {},
SingleImports::None => {},
}

let no_unresolved_invocations =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() => {
Expand Down
49 changes: 49 additions & 0 deletions src/test/ui/issue-50187.rs
Original file line number Diff line number Diff line change
@@ -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 <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.

// 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!();
}