From 96041ccd1054aa05c4ce7f6741df278fbf96c06b Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Fri, 31 Jul 2015 18:58:59 +0200 Subject: [PATCH] More perseverant about indeterminate imports. Most errors generated by resolve might be caused by not-yet-resolved glob imports. This changes the behavior of the resolve imports algorithms to not fail prematurally on first error, but instead buffer intermediate errors and report them only when stuck. Fixes #18083 --- src/librustc_resolve/lib.rs | 12 +++- src/librustc_resolve/resolve_imports.rs | 82 ++++++++++++++++--------- 2 files changed, 63 insertions(+), 31 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index f66dfa83ff948..45dbf08071d19 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -20,6 +20,7 @@ html_root_url = "http://doc.rust-lang.org/nightly/")] #![feature(associated_consts)] +#![feature(borrow_state)] #![feature(rc_weak)] #![feature(rustc_diagnostic_macros)] #![feature(rustc_private)] @@ -538,8 +539,8 @@ enum ResolveResult { } impl ResolveResult { - fn indeterminate(&self) -> bool { - match *self { Indeterminate => true, _ => false } + fn success(&self) -> bool { + match *self { Success(_) => true, _ => false } } } @@ -731,7 +732,12 @@ impl Module { } fn all_imports_resolved(&self) -> bool { - self.imports.borrow().len() == self.resolved_import_count.get() + if self.imports.borrow_state() == ::std::cell::BorrowState::Writing { + // it is currently being resolved ! so nope + false + } else { + self.imports.borrow().len() == self.resolved_import_count.get() + } } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index a9ad0cf137bb9..b368e16d6feac 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -208,7 +208,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { i, self.resolver.unresolved_imports); let module_root = self.resolver.graph_root.get_module(); - self.resolve_imports_for_module_subtree(module_root.clone()); + let errors = self.resolve_imports_for_module_subtree(module_root.clone()); if self.resolver.unresolved_imports == 0 { debug!("(resolving imports) success"); @@ -216,7 +216,20 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { } if self.resolver.unresolved_imports == prev_unresolved_imports { - self.resolver.report_unresolved_imports(module_root); + // resolving failed + if errors.len() > 0 { + for (span, path, help) in errors { + resolve_error(self.resolver, + span, + ResolutionError::UnresolvedImport(Some((&*path, &*help)))); + } + } else { + // report unresolved imports only if no hard error was already reported + // to avoid generating multiple errors on the same import + // imports that are still undeterminate at this point are actually blocked + // by errored imports, so there is no point reporting them + self.resolver.report_unresolved_imports(module_root); + } break; } @@ -227,11 +240,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { /// Attempts to resolve imports for the given module and all of its /// submodules. - fn resolve_imports_for_module_subtree(&mut self, module_: Rc) { + fn resolve_imports_for_module_subtree(&mut self, module_: Rc) + -> Vec<(Span, String, String)> { + let mut errors = Vec::new(); debug!("(resolving imports for module subtree) resolving {}", module_to_string(&*module_)); let orig_module = replace(&mut self.resolver.current_module, module_.clone()); - self.resolve_imports_for_module(module_.clone()); + errors.extend(self.resolve_imports_for_module(module_.clone())); self.resolver.current_module = orig_module; build_reduced_graph::populate_module_if_necessary(self.resolver, &module_); @@ -241,53 +256,65 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { // Nothing to do. } Some(child_module) => { - self.resolve_imports_for_module_subtree(child_module); + errors.extend(self.resolve_imports_for_module_subtree(child_module)); } } } for (_, child_module) in module_.anonymous_children.borrow().iter() { - self.resolve_imports_for_module_subtree(child_module.clone()); + errors.extend(self.resolve_imports_for_module_subtree(child_module.clone())); } + + errors } /// Attempts to resolve imports for the given module only. - fn resolve_imports_for_module(&mut self, module: Rc) { + fn resolve_imports_for_module(&mut self, module: Rc) -> Vec<(Span, String, String)> { + let mut errors = Vec::new(); + if module.all_imports_resolved() { debug!("(resolving imports for module) all imports resolved for \ {}", module_to_string(&*module)); - return; + return errors; } - let imports = module.imports.borrow(); + let mut imports = module.imports.borrow_mut(); let import_count = imports.len(); - while module.resolved_import_count.get() < import_count { + let mut indeterminate_imports = Vec::new(); + while module.resolved_import_count.get() + indeterminate_imports.len() < import_count { let import_index = module.resolved_import_count.get(); - let import_directive = &(*imports)[import_index]; match self.resolve_import_for_module(module.clone(), - import_directive) { + &imports[import_index]) { ResolveResult::Failed(err) => { + let import_directive = &imports[import_index]; let (span, help) = match err { Some((span, msg)) => (span, format!(". {}", msg)), None => (import_directive.span, String::new()) }; - resolve_error(self.resolver, - span, - ResolutionError::UnresolvedImport( - Some((&*import_path_to_string( - &import_directive.module_path, - import_directive.subclass), - &*help))) - ); + errors.push((span, + import_path_to_string( + &import_directive.module_path, + import_directive.subclass + ), + help)) + } + ResolveResult::Indeterminate => {} + ResolveResult::Success(()) => { + // count success + module.resolved_import_count + .set(module.resolved_import_count.get() + 1); + continue; } - ResolveResult::Indeterminate => break, // Bail out. We'll come around next time. - ResolveResult::Success(()) => () // Good. Continue. } + // This resolution was not successful, keep it for later + indeterminate_imports.push(imports.swap_remove(import_index)); - module.resolved_import_count - .set(module.resolved_import_count.get() + 1); } + + imports.extend(indeterminate_imports); + + errors } /// Attempts to resolve the given import. The return value indicates @@ -367,11 +394,10 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { } // Decrement the count of unresolved globs if necessary. But only if - // the resolution result is indeterminate -- otherwise we'll stop - // processing imports here. (See the loop in - // resolve_imports_for_module). + // the resolution result is a success -- other cases will + // be handled by the main loop. - if !resolution_result.indeterminate() { + if resolution_result.success() { match import_directive.subclass { GlobImport => { assert!(module_.glob_count.get() >= 1);