From eb8d9002fa3802ebb78d6854ddec207cbaf6aa18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 May 2020 18:20:37 -0700 Subject: [PATCH 1/7] Add test for #72554. --- src/test/ui/issues/issue-72554.rs | 20 ++++++++++++++++++++ src/test/ui/issues/issue-72554.stderr | 13 +++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 src/test/ui/issues/issue-72554.rs create mode 100644 src/test/ui/issues/issue-72554.stderr diff --git a/src/test/ui/issues/issue-72554.rs b/src/test/ui/issues/issue-72554.rs new file mode 100644 index 0000000000000..47aca05d7786f --- /dev/null +++ b/src/test/ui/issues/issue-72554.rs @@ -0,0 +1,20 @@ +use std::collections::BTreeSet; + +#[derive(Hash)] +pub enum ElemDerived { //~ ERROR recursive type `ElemDerived` has infinite size + A(ElemDerived) +} + +pub enum Elem { + Derived(ElemDerived) +} + +pub struct Set(BTreeSet); + +impl Set { + pub fn into_iter(self) -> impl Iterator { + self.0.into_iter() + } +} + +fn main() {} diff --git a/src/test/ui/issues/issue-72554.stderr b/src/test/ui/issues/issue-72554.stderr new file mode 100644 index 0000000000000..9db65f4a2ee8f --- /dev/null +++ b/src/test/ui/issues/issue-72554.stderr @@ -0,0 +1,13 @@ +error[E0072]: recursive type `ElemDerived` has infinite size + --> $DIR/issue-72554.rs:4:1 + | +LL | pub enum ElemDerived { + | ^^^^^^^^^^^^^^^^^^^^ recursive type has infinite size +LL | A(ElemDerived) + | ----------- recursive without indirection + | + = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `ElemDerived` representable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0072`. From d31d215c56ae4d163e77f8f5a59a4b49859b3d77 Mon Sep 17 00:00:00 2001 From: Patryk Wychowaniec Date: Tue, 2 Jun 2020 20:16:23 +0200 Subject: [PATCH 2/7] Improve E0433, so that it suggests missing imports --- src/librustc_resolve/diagnostics.rs | 11 +- src/librustc_resolve/late.rs | 146 ++++++++++++++---- src/librustc_resolve/lib.rs | 14 +- .../ui/derived-errors/issue-31997-1.stderr | 9 +- src/test/ui/error-codes/E0433.rs | 2 +- src/test/ui/error-codes/E0433.stderr | 6 +- .../ui/hygiene/no_implicit_prelude.stderr | 8 +- src/test/ui/resolve/use_suggestion.rs | 7 + src/test/ui/resolve/use_suggestion.stderr | 42 +++++ 9 files changed, 198 insertions(+), 47 deletions(-) create mode 100644 src/test/ui/resolve/use_suggestion.rs create mode 100644 src/test/ui/resolve/use_suggestion.stderr diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index ea237f1a04f99..cbb2878011c5f 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -1475,7 +1475,7 @@ crate fn show_candidates( // This is `None` if all placement locations are inside expansions use_placement_span: Option, candidates: &[ImportSuggestion], - better: bool, + instead: bool, found_use: bool, ) { if candidates.is_empty() { @@ -1486,6 +1486,7 @@ crate fn show_candidates( // by iterating through a hash map, so make sure they are ordered: let mut path_strings: Vec<_> = candidates.iter().map(|c| path_names_to_string(&c.path)).collect(); + path_strings.sort(); path_strings.dedup(); @@ -1494,8 +1495,9 @@ crate fn show_candidates( } else { ("one of these", "items") }; - let instead = if better { " instead" } else { "" }; - let msg = format!("consider importing {} {}{}", determiner, kind, instead); + + let instead = if instead { " instead" } else { "" }; + let mut msg = format!("consider importing {} {}{}", determiner, kind, instead); if let Some(span) = use_placement_span { for candidate in &mut path_strings { @@ -1507,12 +1509,13 @@ crate fn show_candidates( err.span_suggestions(span, &msg, path_strings.into_iter(), Applicability::Unspecified); } else { - let mut msg = msg; msg.push(':'); + for candidate in path_strings { msg.push('\n'); msg.push_str(&candidate); } + err.note(&msg); } } diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 3b49b3b6ff7d2..f6728b758db65 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -29,8 +29,9 @@ use rustc_span::Span; use smallvec::{smallvec, SmallVec}; use log::debug; +use rustc_span::source_map::{respan, Spanned}; use std::collections::BTreeSet; -use std::mem::replace; +use std::mem::{replace, take}; mod diagnostics; crate mod lifetimes; @@ -234,6 +235,13 @@ impl<'a> PathSource<'a> { } } + fn is_call(self) -> bool { + match self { + PathSource::Expr(Some(&Expr { kind: ExprKind::Call(..), .. })) => true, + _ => false, + } + } + crate fn is_expected(self, res: Res) -> bool { match self { PathSource::Type => match res { @@ -1620,14 +1628,83 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { let report_errors = |this: &mut Self, res: Option| { let (err, candidates) = this.smart_resolve_report_errors(path, span, source, res); + let def_id = this.parent_scope.module.normal_ancestor_id; - let better = res.is_some(); + let instead = res.is_some(); let suggestion = if res.is_none() { this.report_missing_type_error(path) } else { None }; - this.r.use_injections.push(UseError { err, candidates, def_id, better, suggestion }); + + this.r.use_injections.push(UseError { err, candidates, def_id, instead, suggestion }); + PartialRes::new(Res::Err) }; + // For paths originating from calls (like in `HashMap::new()`), tries + // to enrich the plain `failed to resolve: ...` message with hints + // about possible missing imports. + // + // Similar thing, for types, happens in `report_errors` above. + let report_errors_for_call = |this: &mut Self, parent_err: Spanned>| { + if !source.is_call() { + return Some(parent_err); + } + + // Before we start looking for candidates, we have to get our hands + // on the type user is trying to perform invocation on; basically: + // we're transforming `HashMap::new` into just `HashMap` + let path = if let Some((_, path)) = path.split_last() { + path + } else { + return Some(parent_err); + }; + + let (mut err, candidates) = + this.smart_resolve_report_errors(path, span, PathSource::Type, None); + + if candidates.is_empty() { + err.cancel(); + return Some(parent_err); + } + + // There are two different error messages user might receive at + // this point: + // - E0412 cannot find type `{}` in this scope + // - E0433 failed to resolve: use of undeclared type or module `{}` + // + // The first one is emitted for paths in type-position, and the + // latter one - for paths in expression-position. + // + // Thus (since we're in expression-position at this point), not to + // confuse the user, we want to keep the *message* from E04322 (so + // `parent_err`), but we want *hints* from E0412 (so `err`). + // + // And that's what happens below - we're just mixing both messages + // into a single one. + let mut parent_err = this.r.into_struct_error(parent_err.span, parent_err.node); + + parent_err.cancel(); + + err.message = take(&mut parent_err.message); + err.code = take(&mut parent_err.code); + err.children = take(&mut parent_err.children); + + drop(parent_err); + + let def_id = this.parent_scope.module.normal_ancestor_id; + + this.r.use_injections.push(UseError { + err, + candidates, + def_id, + instead: false, + suggestion: None, + }); + + // We don't return `Some(parent_err)` here, because the error will + // be already printed as part of the `use` injections + None + }; + let partial_res = match self.resolve_qpath_anywhere( id, qself, @@ -1637,14 +1714,15 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { source.defer_to_typeck(), crate_lint, ) { - Some(partial_res) if partial_res.unresolved_segments() == 0 => { + Ok(Some(partial_res)) if partial_res.unresolved_segments() == 0 => { if is_expected(partial_res.base_res()) || partial_res.base_res() == Res::Err { partial_res } else { report_errors(self, Some(partial_res.base_res())) } } - Some(partial_res) if source.defer_to_typeck() => { + + Ok(Some(partial_res)) if source.defer_to_typeck() => { // Not fully resolved associated item `T::A::B` or `::A::B` // or `::A::B`. If `B` should be resolved in value namespace then // it needs to be added to the trait map. @@ -1655,25 +1733,34 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } let mut std_path = vec![Segment::from_ident(Ident::with_dummy_span(sym::std))]; + std_path.extend(path); + if self.r.primitive_type_table.primitive_types.contains_key(&path[0].ident.name) { - let cl = CrateLint::No; - let ns = Some(ns); if let PathResult::Module(_) | PathResult::NonModule(_) = - self.resolve_path(&std_path, ns, false, span, cl) + self.resolve_path(&std_path, Some(ns), false, span, CrateLint::No) { - // check if we wrote `str::from_utf8` instead of `std::str::from_utf8` + // Check if we wrote `str::from_utf8` instead of `std::str::from_utf8` let item_span = path.iter().last().map(|segment| segment.ident.span).unwrap_or(span); - debug!("accessed item from `std` submodule as a bare type {:?}", std_path); + let mut hm = self.r.session.confused_type_with_std_module.borrow_mut(); hm.insert(item_span, span); - // In some places (E0223) we only have access to the full path hm.insert(span, span); } } + partial_res } + + Err(err) => { + if let Some(err) = report_errors_for_call(self, err) { + self.r.report_error(err.span, err.node); + } + + PartialRes::new(Res::Err) + } + _ => report_errors(self, None), }; @@ -1682,6 +1769,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // Avoid recording definition of `A::B` in `::B::C`. self.r.record_partial_res(id, partial_res); } + partial_res } @@ -1711,17 +1799,16 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { span: Span, defer_to_typeck: bool, crate_lint: CrateLint, - ) -> Option { + ) -> Result, Spanned>> { let mut fin_res = None; + for (i, ns) in [primary_ns, TypeNS, ValueNS].iter().cloned().enumerate() { if i == 0 || ns != primary_ns { - match self.resolve_qpath(id, qself, path, ns, span, crate_lint) { - // If defer_to_typeck, then resolution > no resolution, - // otherwise full resolution > partial resolution > no resolution. + match self.resolve_qpath(id, qself, path, ns, span, crate_lint)? { Some(partial_res) if partial_res.unresolved_segments() == 0 || defer_to_typeck => { - return Some(partial_res); + return Ok(Some(partial_res)); } partial_res => { if fin_res.is_none() { @@ -1732,19 +1819,19 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } } - // `MacroNS` assert!(primary_ns != MacroNS); + if qself.is_none() { let path_seg = |seg: &Segment| PathSegment::from_ident(seg.ident); let path = Path { segments: path.iter().map(path_seg).collect(), span }; if let Ok((_, res)) = self.r.resolve_macro_path(&path, None, &self.parent_scope, false, false) { - return Some(PartialRes::new(res)); + return Ok(Some(PartialRes::new(res))); } } - fin_res + Ok(fin_res) } /// Handles paths that may refer to associated items. @@ -1756,7 +1843,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ns: Namespace, span: Span, crate_lint: CrateLint, - ) -> Option { + ) -> Result, Spanned>> { debug!( "resolve_qpath(id={:?}, qself={:?}, path={:?}, ns={:?}, span={:?})", id, qself, path, ns, span, @@ -1767,10 +1854,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // This is a case like `::B`, where there is no // trait to resolve. In that case, we leave the `B` // segment to be resolved by type-check. - return Some(PartialRes::with_unresolved_segments( + return Ok(Some(PartialRes::with_unresolved_segments( Res::Def(DefKind::Mod, DefId::local(CRATE_DEF_INDEX)), path.len(), - )); + ))); } // Make sure `A::B` in `::C` is a trait item. @@ -1800,10 +1887,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // The remaining segments (the `C` in our example) will // have to be resolved by type-check, since that requires doing // trait resolution. - return Some(PartialRes::with_unresolved_segments( + return Ok(Some(PartialRes::with_unresolved_segments( partial_res.base_res(), partial_res.unresolved_segments() + path.len() - qself.position - 1, - )); + ))); } let result = match self.resolve_path(&path, Some(ns), true, span, crate_lint) { @@ -1838,11 +1925,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { PartialRes::new(module.res().unwrap()) } PathResult::Failed { is_error_from_last_segment: false, span, label, suggestion } => { - self.r.report_error(span, ResolutionError::FailedToResolve { label, suggestion }); - PartialRes::new(Res::Err) + return Err(respan(span, ResolutionError::FailedToResolve { label, suggestion })); } - PathResult::Module(..) | PathResult::Failed { .. } => return None, - PathResult::Indeterminate => bug!("indetermined path result in resolve_qpath"), + PathResult::Module(..) | PathResult::Failed { .. } => return Ok(None), + PathResult::Indeterminate => bug!("indeterminate path result in resolve_qpath"), }; if path.len() > 1 @@ -1862,7 +1948,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { PathResult::Module(ModuleOrUniformRoot::Module(module)) => { module.res().unwrap() } - _ => return Some(result), + _ => return Ok(Some(result)), } }; if result.base_res() == unqualified_result { @@ -1871,7 +1957,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } } - Some(result) + Ok(Some(result)) } fn with_resolved_label(&mut self, label: Option