Auto merge of #27547 - vberger:more_perseverant_resolve, r=nrc

As noted in my previous PR #27439 , the import resolution algorithm has two cases where it bails out:

- The algorithm will delay an import if the module containing the target of the import still has unresolved glob imports
- The algorithm will delay a glob import of the target module still has unresolved imports

This PR alters the behaviour to only bail out when the above described unresolved imports are `pub`, as non-pub imports don't affect the result anyway.

It is still possible to fail the algorithm with examples like
```rust
pub mod a {
    pub use b::*;
}

pub mod b {
    pub use a::*;
}
```
but such configurations cannot be resolved in any meaningful way, as these are cyclic imports.

Closes #4865

src/librustc_resolve/build_reduced_graph.rs

@@ -924,6 +924,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                                is_public,
                                                                shadowable));
         self.unresolved_imports += 1;
+
+        if is_public {
+            module_.inc_pub_count();
+        }
+
         // Bump the reference count on the name. Or, if this is a glob, set
         // the appropriate flag.
 
@@ -956,7 +961,10 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 // Set the glob flag. This tells us that we don't know the
                 // module's exports ahead of time.
 
-                module_.glob_count.set(module_.glob_count.get() + 1);
+                module_.inc_glob_count();
+                if is_public {
+                    module_.inc_pub_glob_count();
+                }
             }
         }
     }

src/librustc_resolve/lib.rs

@@ -699,6 +699,12 @@ pub struct Module {
     // The number of unresolved globs that this module exports.
     glob_count: Cell<usize>,
 
+    // The number of unresolved pub imports (both regular and globs) in this module
+    pub_count: Cell<usize>,
+
+    // The number of unresolved pub glob imports in this module
+    pub_glob_count: Cell<usize>,
+
     // The index of the import we're resolving.
     resolved_import_count: Cell<usize>,
 
@@ -726,6 +732,8 @@ impl Module {
             anonymous_children: RefCell::new(NodeMap()),
             import_resolutions: RefCell::new(HashMap::new()),
             glob_count: Cell::new(0),
+            pub_count: Cell::new(0),
+            pub_glob_count: Cell::new(0),
             resolved_import_count: Cell::new(0),
             populated: Cell::new(!external),
         }
@@ -741,6 +749,30 @@ impl Module {
     }
 }
 
+impl Module {
+    pub fn inc_glob_count(&self) {
+        self.glob_count.set(self.glob_count.get() + 1);
+    }
+    pub fn dec_glob_count(&self) {
+        assert!(self.glob_count.get() > 0);
+        self.glob_count.set(self.glob_count.get() - 1);
+    }
+    pub fn inc_pub_count(&self) {
+        self.pub_count.set(self.pub_count.get() + 1);
+    }
+    pub fn dec_pub_count(&self) {
+        assert!(self.pub_count.get() > 0);
+        self.pub_count.set(self.pub_count.get() - 1);
+    }
+    pub fn inc_pub_glob_count(&self) {
+        self.pub_glob_count.set(self.pub_glob_count.get() + 1);
+    }
+    pub fn dec_pub_glob_count(&self) {
+        assert!(self.pub_glob_count.get() > 0);
+        self.pub_glob_count.set(self.pub_glob_count.get() - 1);
+    }
+}
+
 impl fmt::Debug for Module {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{:?}, kind: {:?}, {}",

src/librustc_resolve/resolve_imports.rs

@@ -407,13 +407,18 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         if resolution_result.success() {
             match import_directive.subclass {
                 GlobImport => {
-                    assert!(module_.glob_count.get() >= 1);
+                    module_.dec_glob_count();
-                    module_.glob_count.set(module_.glob_count.get() - 1);
+                    if import_directive.is_public {
+                        module_.dec_pub_glob_count();
+                    }
                 }
                 SingleImport(..) => {
                     // Ignore.
                 }
             }
+            if import_directive.is_public {
+                module_.dec_pub_count();
+            }
         }
 
         return resolution_result;
@@ -503,8 +508,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 {
+                if target_module.pub_glob_count.get() > 0 {
-                    debug!("(resolving single import) unresolved glob; \
+                    debug!("(resolving single import) unresolved pub glob; \
                             bailing out");
                     return ResolveResult::Indeterminate;
                 }
@@ -767,16 +772,26 @@ 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
+            debug!("(resolving glob imports) target module is current module; giving up");
+            return ResolveResult::Failed(Some((
+                        import_directive.span,
+                        "Cannot glob-import a module into itself.".into()
+                    )));
+        }
+
         for (ident, target_import_resolution) in import_resolutions.iter() {
             debug!("(resolving glob import) writing module resolution \
                     {} into `{}`",

src/test/compile-fail/issue-25396.rs

@@ -11,14 +11,14 @@
 use foo::baz;
 use bar::baz; //~ ERROR a module named `baz` has already been imported
 
-use bar::Quux; //~ ERROR a trait named `Quux` has already been imported
 use foo::Quux;
+use bar::Quux; //~ ERROR a trait named `Quux` has already been imported
 
-use foo::blah; //~ ERROR a type named `blah` has already been imported
+use foo::blah;
-use bar::blah;
+use bar::blah; //~ ERROR a type named `blah` has already been imported
 
-use foo::WOMP; //~ ERROR a value named `WOMP` has already been imported
+use foo::WOMP;
-use bar::WOMP;
+use bar::WOMP; //~ ERROR a value named `WOMP` has already been imported
 
 fn main() {}
 

src/test/compile-fail/issue-8208.rs

@@ -8,7 +8,17 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use self::*; //~ ERROR: unresolved import
+use self::*; //~ ERROR: unresolved import `self::*`. Cannot glob-import a module into itself.
+
+mod foo {
+    use foo::*; //~ ERROR: unresolved import `foo::*`. Cannot glob-import a module into itself.
+
+    mod bar {
+        use super::bar::*;
+        //~^ ERROR: unresolved import `super::bar::*`. Cannot glob-import a module into itself.
+    }
+
+}
 
 fn main() {
 }

src/test/run-pass/issue-4865-2.rs

@@ -0,0 +1,33 @@
+// 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.
+
+// Previously, this would have failed to resolve due to the circular
+// block between `use say` and `pub use hello::*`.
+//
+// Now, as `use say` is not `pub`, the glob import can resolve
+// without any problem and this resolves fine.
+
+pub use hello::*;
+
+pub mod say {
+    pub fn hello() { println!("hello"); }
+}
+
+pub mod hello {
+    use say;
+
+    pub fn hello() {
+        say::hello();
+    }
+}
+
+fn main() {
+    hello();
+}

src/test/run-pass/issue-4865-3.rs

@@ -0,0 +1,25 @@
+// 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.
+
+// This should resolve fine even with the circular imports as
+// they are not `pub`.
+
+pub mod a {
+    use b::*;
+}
+
+pub mod b {
+    use a::*;
+}
+
+use a::*;
+
+fn main() {
+}