Skip to content

Handle conflicting import of items declared in the same module #19522

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.

Sign up for GitHub

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
Dec 7, 2014
Merged

Handle conflicting import of items declared in the same module #19522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7403b0c
Handle conflicting import of items declared in the same module
mukilan Dec 4, 2014
4b75a5d
Add compile-fail tests for #19498
mukilan Dec 7, 2014
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
39 changes: 32 additions & 7 deletions src/librustc/middle/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2654,10 +2654,34 @@ impl<'a> Resolver<'a> {

}
Some(_) => {
// The import is unresolved. Bail out.
debug!("(resolving single import) unresolved import; \
bailing out");
return Indeterminate;
// If containing_module is the same module whose import we are resolving
// and there it has an unresolved import with the same name as `source`,
// then the user is actually trying to import an item that is declared
// in the same scope
//
// e.g
// use self::submodule;
// pub mod submodule;
//
// In this case we continue as if we resolved the import and let the
// check_for_conflicts_between_imports_and_items call below handle
// the conflict
match (module_.def_id.get(), containing_module.def_id.get()) {
(Some(id1), Some(id2)) if id1 == id2 => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this missing some cases - sorry to be vague, but it seems like you are only catching reflexive loops, but any loop could cause this issue (is that right?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nick29581 : I assume you are talking about cases like this (the below one is a simple loop, while we can have arbitrary length cycles)

mod A {
   pub use B::submodule;  // error: unresolved import B::submodule;
}

mod B {
   pub use A::submodule; // error: unresolved import A::submodule;
}

fn main() {
}

In such cases, the fixed point algorithm will terminate because no progress will be made in resolving any of the import directive, and an error will be printed for each import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nick29581 : I think now I understand your concern. The existing logic takes care of case like this.

mod A {

   use self::B::C::B;  //  error: import `B` conflicts with existing submodule

   mod B {   //  note: note conflicting module here
       mod C {
            mod B {}
       }
   }
}


fn main() {
}

if value_result.is_unknown() {
value_result = UnboundResult;
}
if type_result.is_unknown() {
type_result = UnboundResult;
}
}
_ => {
// The import is unresolved. Bail out.
debug!("(resolving single import) unresolved import; \
bailing out");
return Indeterminate;
}
}
}
}
}
Expand Down Expand Up @@ -3018,7 +3042,7 @@ impl<'a> Resolver<'a> {
fn check_for_conflicts_between_imports_and_items(&mut self,
module: &Module,
import_resolution:
&mut ImportResolution,
&ImportResolution,
import_span: Span,
name: Name) {
if self.session.features.borrow().import_shadowing {
Expand All @@ -3031,8 +3055,9 @@ impl<'a> Resolver<'a> {
.contains_key(&name) {
match import_resolution.type_target {
Some(ref target) if !target.shadowable => {
let msg = format!("import `{}` conflicts with imported \
crate in this module",
let msg = format!("import `{0}` conflicts with imported \
crate in this module \
(maybe you meant `use {0}::*`?)",
token::get_name(name).get());
self.session.span_err(import_span, msg.as_slice());
}
Expand Down
21 changes: 21 additions & 0 deletions src/test/compile-fail/issue-19498.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2013 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.

use self::A; //~ ERROR import `A` conflicts with existing submodule
use self::B; //~ ERROR import `B` conflicts with existing submodule
mod A {}
pub mod B {}

mod C {
use C::D; //~ ERROR import `D` conflicts with existing submodule
mod D {}
}

fn main() {}
3 changes: 2 additions & 1 deletion src/test/compile-fail/unresolved-extern-mod-suggestion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

extern crate core;
use core; //~ ERROR unresolved import (maybe you meant `core::*`?)
use core;
//~^ ERROR import `core` conflicts with imported crate in this module

fn main() {}