From 18130ef044cfa83af341c4a3500ab576f8cc4c38 Mon Sep 17 00:00:00 2001 From: Mark Rousskov <mark.simulacrum@gmail.com> Date: Mon, 5 Aug 2019 10:21:25 -0400 Subject: [PATCH 1/2] Replace error callback with Result --- src/librustc_resolve/lib.rs | 47 +++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1a203e73f0a86..3dd1d7d274d5a 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1771,8 +1771,13 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { path: &ast::Path, is_value: bool, ) -> Res { - self.resolve_ast_path_cb(path, is_value, - |resolver, span, error| resolve_error(resolver, span, error)) + match self.resolve_ast_path_inner(path, is_value) { + Ok(r) => r, + Err((span, error)) => { + resolve_error(self, span, error); + Res::Err + } + } } fn resolve_str_path( @@ -1833,8 +1838,6 @@ impl<'a> Resolver<'a> { /// just that an error occurred. pub fn resolve_str_path_error(&mut self, span: Span, path_str: &str, is_value: bool) -> Result<(ast::Path, Res), ()> { - let mut errored = false; - let path = if path_str.starts_with("::") { ast::Path { span, @@ -1855,24 +1858,24 @@ impl<'a> Resolver<'a> { .collect(), } }; - let res = self.resolve_ast_path_cb(&path, is_value, |_, _, _| errored = true); - if errored || res == def::Res::Err { - Err(()) - } else { - Ok((path, res)) + match self.resolve_ast_path_inner(&path, is_value) { + Ok(res) => { + if res == Res::Err { + Err(()) + } else { + Ok((path, res)) + } + } + Err(_) => Err(()), } } /// Like `resolve_ast_path`, but takes a callback in case there was an error. - // FIXME(eddyb) use `Result` or something instead of callbacks. - fn resolve_ast_path_cb<F>( + fn resolve_ast_path_inner( &mut self, path: &ast::Path, is_value: bool, - error_callback: F, - ) -> Res - where F: for<'c, 'b> FnOnce(&'c mut Resolver<'_>, Span, ResolutionError<'b>) - { + ) -> Result<Res, (Span, ResolutionError<'a>)> { let namespace = if is_value { ValueNS } else { TypeNS }; let span = path.span; let path = Segment::from_path(&path); @@ -1880,23 +1883,21 @@ impl<'a> Resolver<'a> { match self.resolve_path_without_parent_scope(&path, Some(namespace), true, span, CrateLint::No) { PathResult::Module(ModuleOrUniformRoot::Module(module)) => - module.res().unwrap(), + Ok(module.res().unwrap()), PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 => - path_res.base_res(), + Ok(path_res.base_res()), PathResult::NonModule(..) => { - error_callback(self, span, ResolutionError::FailedToResolve { + Err((span, ResolutionError::FailedToResolve { label: String::from("type-relative paths are not supported in this context"), suggestion: None, - }); - Res::Err + })) } PathResult::Module(..) | PathResult::Indeterminate => unreachable!(), PathResult::Failed { span, label, suggestion, .. } => { - error_callback(self, span, ResolutionError::FailedToResolve { + Err((span, ResolutionError::FailedToResolve { label, suggestion, - }); - Res::Err + })) } } } From 3cd7f08ed1f801c2fa4983d9eef9162739922373 Mon Sep 17 00:00:00 2001 From: Mark Rousskov <mark.simulacrum@gmail.com> Date: Mon, 5 Aug 2019 12:25:32 -0400 Subject: [PATCH 2/2] Force callers of resolve_ast_path to deal with Res::Err correctly --- src/librustc_resolve/lib.rs | 12 ++---------- src/librustdoc/passes/collect_intra_doc_links.rs | 7 +++++++ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 3dd1d7d274d5a..ba70355899879 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1858,16 +1858,8 @@ impl<'a> Resolver<'a> { .collect(), } }; - match self.resolve_ast_path_inner(&path, is_value) { - Ok(res) => { - if res == Res::Err { - Err(()) - } else { - Ok((path, res)) - } - } - Err(_) => Err(()), - } + let res = self.resolve_ast_path_inner(&path, is_value).map_err(|_| ())?; + Ok((path, res)) } /// Like `resolve_ast_path`, but takes a callback in case there was an error. diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 04de3374d0587..84cfdd790b733 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -71,6 +71,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns == ValueNS) }) }); + let result = match result { + Ok((_, Res::Err)) => Err(()), + _ => result, + }; if let Ok((_, res)) = result { let res = res.map_id(|_| panic!("unexpected node_id")); @@ -134,6 +138,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let (_, ty_res) = cx.enter_resolver(|resolver| resolver.with_scope(node_id, |resolver| { resolver.resolve_str_path_error(DUMMY_SP, &path, false) }))?; + if let Res::Err = ty_res { + return Err(()); + } let ty_res = ty_res.map_id(|_| panic!("unexpected node_id")); match ty_res { Res::Def(DefKind::Struct, did)