-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Block import resolution only on 'pub' imports #27547
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
Changes from 2 commits
b69bf11
7516759
8e24091
3d041bd
5847ea7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -926,6 +926,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { | |
is_public, | ||
shadowable)); | ||
self.unresolved_imports += 1; | ||
|
||
if is_public { | ||
module_.pub_count.set(module_.pub_count.get() + 1); | ||
} | ||
|
||
// Bump the reference count on the name. Or, if this is a glob, set | ||
// the appropriate flag. | ||
|
||
|
@@ -959,6 +964,9 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { | |
// module's exports ahead of time. | ||
|
||
module_.glob_count.set(module_.glob_count.get() + 1); | ||
if is_public { | ||
module_.pub_glob_count.set(module_.pub_glob_count.get() + 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. likewise |
||
} | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -409,11 +409,19 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { | |
GlobImport => { | ||
assert!(module_.glob_count.get() >= 1); | ||
module_.glob_count.set(module_.glob_count.get() - 1); | ||
if import_directive.is_public { | ||
assert!(module_.pub_glob_count.get() >= 1); | ||
module_.pub_glob_count.set(module_.pub_glob_count.get() - 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and for decrements |
||
} | ||
} | ||
SingleImport(..) => { | ||
// Ignore. | ||
} | ||
} | ||
if import_directive.is_public { | ||
assert!(module_.pub_count.get() >= 1); | ||
module_.pub_count.set(module_.pub_count.get() - 1); | ||
} | ||
} | ||
|
||
return resolution_result; | ||
|
@@ -503,8 +511,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { | |
// containing module, bail out. We don't know enough to be | ||
// able to resolve this import. | ||
|
||
if target_module.glob_count.get() > 0 { | ||
debug!("(resolving single import) unresolved glob; \ | ||
if target_module.pub_glob_count.get() > 0 { | ||
debug!("(resolving single import) unresolved pub glob; \ | ||
bailing out"); | ||
return ResolveResult::Indeterminate; | ||
} | ||
|
@@ -767,16 +775,22 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { | |
|
||
// We must bail out if the node has unresolved imports of any kind | ||
// (including globs). | ||
if !(*target_module).all_imports_resolved() { | ||
if (*target_module).pub_count.get() > 0 { | ||
debug!("(resolving glob import) target module has unresolved \ | ||
imports; bailing out"); | ||
pub imports; bailing out"); | ||
return ResolveResult::Indeterminate; | ||
} | ||
|
||
assert_eq!(target_module.glob_count.get(), 0); | ||
|
||
// Add all resolved imports from the containing module. | ||
let import_resolutions = target_module.import_resolutions.borrow(); | ||
|
||
if module_.import_resolutions.borrow_state() != ::std::cell::BorrowState::Unused { | ||
// In this case, target_module == module_ | ||
// This means we are trying to glob import a module into itself, | ||
// and it is a no-go | ||
return ResolveResult::Indeterminate; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a specific warning for this case? |
||
} | ||
|
||
for (ident, target_import_resolution) in import_resolutions.iter() { | ||
debug!("(resolving glob import) writing module resolution \ | ||
{} into `{}`", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// Copyright 2015 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. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tests need comments please |
||
pub use hello::*; | ||
|
||
pub mod say { | ||
pub fn hello() { println!("hello"); } | ||
} | ||
|
||
pub mod hello { | ||
use say; | ||
|
||
pub fn hello() { | ||
say::hello(); | ||
} | ||
} | ||
|
||
fn main() { | ||
hello(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
// Copyright 2015 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. | ||
|
||
pub mod a { | ||
use b::*; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: indent |
||
} | ||
|
||
pub mod b { | ||
use a::*; | ||
} | ||
|
||
use a::*; | ||
|
||
fn main() { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you factor this out into an inc_pub_count method?