Skip to content

Commit

Permalink
Fix ICE caused by suggestion with no code substitutions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
estebank committed Feb 14, 2021
1 parent 178108b commit 04c2454
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 20 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -318,6 +319,10 @@ impl Diagnostic {
suggestions: Vec<Vec<(Span, String)>>,
applicability: Applicability,
) -> &mut Self {
assert!(!suggestions.is_empty());
for s in &suggestions {
assert!(!s.is_empty());
}
self.suggestions.push(CodeSuggestion {
substitutions: suggestions
.into_iter()
Expand Down Expand Up @@ -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
Expand Down
28 changes: 8 additions & 20 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,13 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
.collect::<Vec<_>>();
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())
Expand All @@ -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::<Vec<_>>();
enum_candidates.sort();

// Contextualize for E0412 "cannot find type", but don't belabor the point
Expand All @@ -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,
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -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<T, E> {
| --------------------- 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`.

0 comments on commit 04c2454

Please sign in to comment.