From 04c2454b1ecdf8e8611f291708ef38937288a250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 13 Feb 2021 19:52:12 -0800 Subject: [PATCH] Fix ICE caused by suggestion with no code substitutions Change suggestion logic to filter and checking _before_ creating specific resolution suggestion. Assert earlier that suggestions contain code substitions to make it easier in the future to debug invalid uses. If we find this becomes too noisy in the wild, we can always make the emitter resilient to these cases and remove the assertions. Fix #78651. --- compiler/rustc_errors/src/diagnostic.rs | 6 ++++ .../rustc_resolve/src/late/diagnostics.rs | 28 ++++++------------- ...empt-to-add-suggestions-with-no-changes.rs | 5 ++++ ...-to-add-suggestions-with-no-changes.stderr | 20 +++++++++++++ 4 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.rs create mode 100644 src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.stderr diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index ef4a45cab4135..ce5b130dd97fe 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -295,6 +295,7 @@ impl Diagnostic { suggestion: Vec<(Span, String)>, applicability: Applicability, ) -> &mut Self { + assert!(!suggestion.is_empty()); self.suggestions.push(CodeSuggestion { substitutions: vec![Substitution { parts: suggestion @@ -318,6 +319,10 @@ impl Diagnostic { suggestions: Vec>, applicability: Applicability, ) -> &mut Self { + assert!(!suggestions.is_empty()); + for s in &suggestions { + assert!(!s.is_empty()); + } self.suggestions.push(CodeSuggestion { substitutions: suggestions .into_iter() @@ -348,6 +353,7 @@ impl Diagnostic { suggestion: Vec<(Span, String)>, applicability: Applicability, ) -> &mut Self { + assert!(!suggestion.is_empty()); self.suggestions.push(CodeSuggestion { substitutions: vec![Substitution { parts: suggestion diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index d2a693a568828..717891b0f87e9 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -319,9 +319,13 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { .collect::>(); let crate_def_id = DefId::local(CRATE_DEF_INDEX); if candidates.is_empty() && is_expected(Res::Def(DefKind::Enum, crate_def_id)) { - let enum_candidates = - self.r.lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant); - + let mut enum_candidates: Vec<_> = self + .r + .lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant) + .into_iter() + .map(|suggestion| import_candidate_to_enum_paths(&suggestion)) + .filter(|(_, enum_ty_path)| enum_ty_path != "std::prelude::v1") + .collect(); if !enum_candidates.is_empty() { if let (PathSource::Type, Some(span)) = (source, self.diagnostic_metadata.current_type_ascription.last()) @@ -340,10 +344,6 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { } } - let mut enum_candidates = enum_candidates - .iter() - .map(|suggestion| import_candidate_to_enum_paths(&suggestion)) - .collect::>(); enum_candidates.sort(); // Contextualize for E0412 "cannot find type", but don't belabor the point @@ -363,19 +363,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { err.span_suggestions( span, &msg, - enum_candidates - .into_iter() - .map(|(_variant_path, enum_ty_path)| enum_ty_path) - // Variants re-exported in prelude doesn't mean `prelude::v1` is the - // type name! - // FIXME: is there a more principled way to do this that - // would work for other re-exports? - .filter(|enum_ty_path| enum_ty_path != "std::prelude::v1") - // Also write `Option` rather than `std::prelude::v1::Option`. - .map(|enum_ty_path| { - // FIXME #56861: DRY-er prelude filtering. - enum_ty_path.trim_start_matches("std::prelude::v1::").to_owned() - }), + enum_candidates.into_iter().map(|(_variant_path, enum_ty_path)| enum_ty_path), Applicability::MachineApplicable, ); } diff --git a/src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.rs b/src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.rs new file mode 100644 index 0000000000000..a25be862a5daf --- /dev/null +++ b/src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.rs @@ -0,0 +1,5 @@ +use std::result; +impl result { //~ ERROR expected type, found module `result` + fn into_future() -> Err {} //~ ERROR expected type, found variant `Err` +} +fn main() {} diff --git a/src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.stderr b/src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.stderr new file mode 100644 index 0000000000000..59ccb29e0901a --- /dev/null +++ b/src/test/ui/suggestions/do-not-attempt-to-add-suggestions-with-no-changes.stderr @@ -0,0 +1,20 @@ +error[E0573]: expected type, found module `result` + --> $DIR/do-not-attempt-to-add-suggestions-with-no-changes.rs:2:6 + | +LL | impl result { + | ^^^^^^ help: an enum with a similar name exists: `Result` + | + ::: $SRC_DIR/core/src/result.rs:LL:COL + | +LL | pub enum Result { + | --------------------- similarly named enum `Result` defined here + +error[E0573]: expected type, found variant `Err` + --> $DIR/do-not-attempt-to-add-suggestions-with-no-changes.rs:3:25 + | +LL | fn into_future() -> Err {} + | ^^^ not a type + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0573`.