Skip to content

Add name bindings for bad imports #31338

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
Feb 3, 2016
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
2 changes: 1 addition & 1 deletion src/librustc/middle/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl Def {
Def::TyParam(..) | Def::Struct(..) | Def::Trait(..) |
Def::Method(..) | Def::Const(..) | Def::AssociatedConst(..) |
Def::PrimTy(..) | Def::Label(..) | Def::SelfTy(..) | Def::Err => {
panic!("attempted .def_id() on invalid {:?}", self)
panic!("attempted .var_id() on invalid {:?}", self)
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1468,7 +1468,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
match search_module.parent_link {
NoParentLink => {
// No more parents. This module was unresolved.
debug!("(resolving item in lexical scope) unresolved module");
debug!("(resolving item in lexical scope) unresolved module: no parent module");
return Failed(None);
}
ModuleParentLink(parent_module_node, _) => {
Expand Down Expand Up @@ -3109,7 +3109,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
Indeterminate => None,
Failed(err) => {
debug!("(resolving item path by identifier in lexical scope) failed to resolve {}",
debug!("(resolving item path by identifier in lexical scope) failed to \
resolve `{}`",
name);

if let Some((span, msg)) = err {
Expand Down
67 changes: 56 additions & 11 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use self::ImportDirectiveSubclass::*;

use DefModifiers;
use DefOrModule;
use Module;
use Namespace::{self, TypeNS, ValueNS};
use NameBinding;
Expand Down Expand Up @@ -50,7 +51,7 @@ pub enum Shadowable {
}

/// One import directive.
#[derive(Debug)]
#[derive(Debug,Clone)]
pub struct ImportDirective {
pub module_path: Vec<Name>,
pub subclass: ImportDirectiveSubclass,
Expand Down Expand Up @@ -140,9 +141,11 @@ impl<'a> ImportResolution<'a> {
}
}

struct ImportResolvingError {
struct ImportResolvingError<'a> {
/// Module where the error happened
source_module: Module<'a>,
import_directive: ImportDirective,
span: Span,
path: String,
help: String,
}

Expand Down Expand Up @@ -181,9 +184,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
// resolving failed
if errors.len() > 0 {
for e in errors {
resolve_error(self.resolver,
e.span,
ResolutionError::UnresolvedImport(Some((&e.path, &e.help))));
self.import_resolving_error(e)
}
} else {
// Report unresolved imports only if no hard error was already reported
Expand All @@ -200,11 +201,55 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
}
}

/// Resolves an `ImportResolvingError` into the correct enum discriminant
/// and passes that on to `resolve_error`.
fn import_resolving_error(&self, e: ImportResolvingError) {
// If it's a single failed import then create a "fake" import
// resolution for it so that later resolve stages won't complain.
if let SingleImport(target, _) = e.import_directive.subclass {
let mut import_resolutions = e.source_module.import_resolutions.borrow_mut();

let resolution = import_resolutions.entry((target, ValueNS)).or_insert_with(|| {
debug!("(resolving import error) adding import resolution for `{}`",
target);

ImportResolution::new(e.import_directive.id,
e.import_directive.is_public)
});

if resolution.target.is_none() {
debug!("(resolving import error) adding fake target to import resolution of `{}`",
target);

let name_binding = NameBinding {
modifiers: DefModifiers::IMPORTABLE,
def_or_module: DefOrModule::Def(Def::Err),
span: None,
};

// Create a fake target pointing to a fake name binding in our
// own module
let target = Target::new(e.source_module,
name_binding,
Shadowable::Always);

resolution.target = Some(target);
}
}

let path = import_path_to_string(&e.import_directive.module_path,
e.import_directive.subclass);

resolve_error(self.resolver,
e.span,
ResolutionError::UnresolvedImport(Some((&path, &e.help))));
}

/// Attempts to resolve imports for the given module and all of its
/// submodules.
fn resolve_imports_for_module_subtree(&mut self,
module_: Module<'b>)
-> Vec<ImportResolvingError> {
-> Vec<ImportResolvingError<'b>> {
let mut errors = Vec::new();
debug!("(resolving imports for module subtree) resolving {}",
module_to_string(&*module_));
Expand Down Expand Up @@ -232,7 +277,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
}

/// Attempts to resolve imports for the given module only.
fn resolve_imports_for_module(&mut self, module: Module<'b>) -> Vec<ImportResolvingError> {
fn resolve_imports_for_module(&mut self, module: Module<'b>) -> Vec<ImportResolvingError<'b>> {
let mut errors = Vec::new();

if module.all_imports_resolved() {
Expand All @@ -254,9 +299,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
None => (import_directive.span, String::new()),
};
errors.push(ImportResolvingError {
source_module: module,
import_directive: import_directive.clone(),
span: span,
path: import_path_to_string(&import_directive.module_path,
import_directive.subclass),
help: help,
});
}
Expand Down Expand Up @@ -784,7 +829,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
namespace_name,
name);
span_err!(self.resolver.session, import_directive.span, E0251, "{}", msg);
} else {
} else {
let target = Target::new(containing_module,
name_binding.clone(),
import_directive.shadowable);
Expand Down
7 changes: 5 additions & 2 deletions src/test/compile-fail/import-from-missing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@ mod spam {
pub fn ham() { }
}

fn main() { ham(); eggs(); }
//~^ ERROR unresolved name `eggs`
fn main() {
ham();
// Expect eggs to pass because the compiler inserts a fake name for it
eggs();
}
6 changes: 3 additions & 3 deletions src/test/compile-fail/import2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
use baz::zed::bar;
//~^ ERROR unresolved import `baz::zed::bar`. Could not find `zed` in `baz`


mod baz {}
mod zed {
pub fn bar() { println!("bar3"); }
}
fn main() { bar(); }
//~^ ERROR unresolved name `bar`
fn main() {
bar();
}
4 changes: 3 additions & 1 deletion src/test/compile-fail/privacy3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ pub fn foo() {}
fn test1() {
use bar::gpriv;
//~^ ERROR unresolved import `bar::gpriv`. There is no `gpriv` in `bar`

// This should pass because the compiler will insert a fake name binding
// for `gpriv`
gpriv();
//~^ ERROR unresolved name `gpriv`
}

#[start] fn main(_: isize, _: *const *const u8) -> isize { 3 }