From 121d9f5b16cae379c6b57e27ff14f25e431c44fa Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 8 Nov 2023 05:13:35 +0000 Subject: [PATCH 1/3] make LayoutError::Cycle carry ErrorGuaranteed --- compiler/rustc_middle/src/ty/layout.rs | 10 +++++----- compiler/rustc_middle/src/values.rs | 5 ++--- src/librustdoc/html/templates/type_layout.html | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 4223e503f5e22..aca6acd783b92 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -215,7 +215,7 @@ pub enum LayoutError<'tcx> { SizeOverflow(Ty<'tcx>), NormalizationFailure(Ty<'tcx>, NormalizationError<'tcx>), ReferencesError(ErrorGuaranteed), - Cycle, + Cycle(ErrorGuaranteed), } impl<'tcx> LayoutError<'tcx> { @@ -226,7 +226,7 @@ impl<'tcx> LayoutError<'tcx> { Unknown(_) => middle_unknown_layout, SizeOverflow(_) => middle_values_too_big, NormalizationFailure(_, _) => middle_cannot_be_normalized, - Cycle => middle_cycle, + Cycle(_) => middle_cycle, ReferencesError(_) => middle_layout_references_error, } } @@ -240,7 +240,7 @@ impl<'tcx> LayoutError<'tcx> { NormalizationFailure(ty, e) => { E::NormalizationFailure { ty, failure_ty: e.get_type_for_failure() } } - Cycle => E::Cycle, + Cycle(_) => E::Cycle, ReferencesError(_) => E::ReferencesError, } } @@ -261,7 +261,7 @@ impl<'tcx> fmt::Display for LayoutError<'tcx> { t, e.get_type_for_failure() ), - LayoutError::Cycle => write!(f, "a cycle occurred during layout computation"), + LayoutError::Cycle(_) => write!(f, "a cycle occurred during layout computation"), LayoutError::ReferencesError(_) => write!(f, "the type has an unknown layout"), } } @@ -333,7 +333,7 @@ impl<'tcx> SizeSkeleton<'tcx> { Err(err @ LayoutError::Unknown(_)) => err, // We can't extract SizeSkeleton info from other layout errors Err( - e @ LayoutError::Cycle + e @ LayoutError::Cycle(_) | e @ LayoutError::SizeOverflow(_) | e @ LayoutError::NormalizationFailure(..) | e @ LayoutError::ReferencesError(_), diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index f30993c9a694d..2b4ae37362699 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -114,12 +114,11 @@ impl<'tcx> Value> for ty::EarlyBinder> } impl<'tcx, T> Value> for Result> { - fn from_cycle_error(_tcx: TyCtxt<'tcx>, _cycle: &[QueryInfo], _guar: ErrorGuaranteed) -> Self { + fn from_cycle_error(_tcx: TyCtxt<'tcx>, _cycle: &[QueryInfo], guar: ErrorGuaranteed) -> Self { // tcx.arena.alloc cannot be used because we are not allowed to use &'tcx LayoutError under // min_specialization. Since this is an error path anyways, leaking doesn't matter (and really, // tcx.arena.alloc is pretty much equal to leaking). - // FIXME: `Cycle` should carry the ErrorGuaranteed - Err(Box::leak(Box::new(ty::layout::LayoutError::Cycle))) + Err(Box::leak(Box::new(ty::layout::LayoutError::Cycle(guar)))) } } diff --git a/src/librustdoc/html/templates/type_layout.html b/src/librustdoc/html/templates/type_layout.html index b8b7785a2a191..c75fc34a9dfe7 100644 --- a/src/librustdoc/html/templates/type_layout.html +++ b/src/librustdoc/html/templates/type_layout.html @@ -54,7 +54,7 @@

{# #} Note: Encountered an error during type layout; {#+ #} the type failed to be normalized. {# #}

{# #} - {% when Err(LayoutError::Cycle) %} + {% when Err(LayoutError::Cycle(_)) %}

{# #} Note: Encountered an error during type layout; {#+ #} the type's layout depended on the type's layout itself. {# #} From 69634f2077bc4f223cca7dfb63e104ac1efa0752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 13 Nov 2023 00:52:10 +0000 Subject: [PATCH 2/3] Always point at index span on index obligation failure Use more targetted span for index obligation failures by rewriting the obligation cause span. CC #66023 --- compiler/rustc_hir_typeck/src/expr.rs | 14 ++------------ .../point-at-index-for-obligation-failure.rs | 7 +++++++ .../point-at-index-for-obligation-failure.stderr | 13 +++++++++++++ 3 files changed, 22 insertions(+), 12 deletions(-) create mode 100644 tests/ui/indexing/point-at-index-for-obligation-failure.rs create mode 100644 tests/ui/indexing/point-at-index-for-obligation-failure.stderr diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 9f439a2b32aaf..7298e1e570221 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -2877,7 +2877,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // two-phase not needed because index_ty is never mutable self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No); self.select_obligations_where_possible(|errors| { - self.point_at_index_if_possible(errors, idx.span) + self.point_at_index(errors, idx.span) }); element_ty } @@ -3036,18 +3036,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .ok() } - fn point_at_index_if_possible( - &self, - errors: &mut Vec>, - span: Span, - ) { + fn point_at_index(&self, errors: &mut Vec>, span: Span) { for error in errors { - match error.obligation.predicate.kind().skip_binder() { - ty::PredicateKind::Clause(ty::ClauseKind::Trait(predicate)) - if self.tcx.is_diagnostic_item(sym::SliceIndex, predicate.trait_ref.def_id) => { - } - _ => continue, - } error.obligation.cause.span = span; } } diff --git a/tests/ui/indexing/point-at-index-for-obligation-failure.rs b/tests/ui/indexing/point-at-index-for-obligation-failure.rs new file mode 100644 index 0000000000000..e9c429b53ced1 --- /dev/null +++ b/tests/ui/indexing/point-at-index-for-obligation-failure.rs @@ -0,0 +1,7 @@ +fn main() { + let a = std::collections::HashMap::::new(); + let s = "hello"; + let _b = a[ + &s //~ ERROR E0277 + ]; +} diff --git a/tests/ui/indexing/point-at-index-for-obligation-failure.stderr b/tests/ui/indexing/point-at-index-for-obligation-failure.stderr new file mode 100644 index 0000000000000..3e2fbc2ab6f2e --- /dev/null +++ b/tests/ui/indexing/point-at-index-for-obligation-failure.stderr @@ -0,0 +1,13 @@ +error[E0277]: the trait bound `String: Borrow<&str>` is not satisfied + --> $DIR/point-at-index-for-obligation-failure.rs:5:9 + | +LL | &s + | ^^ the trait `Borrow<&str>` is not implemented for `String` + | + = help: the trait `Borrow` is implemented for `String` + = help: for that trait implementation, expected `str`, found `&str` + = note: required for `HashMap` to implement `Index<&&str>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From 5061c09c58948de2c09ea7d8f3b50b994608dd7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 13 Nov 2023 17:05:08 +0000 Subject: [PATCH 3/3] review comments: more targeted span setting approach --- compiler/rustc_hir_typeck/src/expr.rs | 26 +++++++++++++++++-- .../indexing/indexing-requires-a-uint.stderr | 2 ++ .../suggest-dereferencing-index.stderr | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 7298e1e570221..f1987c29ddcc1 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -21,7 +21,7 @@ use crate::{ TupleArgumentsFlag::DontTupleArguments, }; use rustc_ast as ast; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{ pluralize, struct_span_err, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, @@ -2877,7 +2877,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // two-phase not needed because index_ty is never mutable self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No); self.select_obligations_where_possible(|errors| { - self.point_at_index(errors, idx.span) + self.point_at_index(errors, idx.span); }); element_ty } @@ -3037,7 +3037,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } fn point_at_index(&self, errors: &mut Vec>, span: Span) { + let mut seen_preds = FxHashSet::default(); + // We re-sort here so that the outer most root obligations comes first, as we have the + // subsequent weird logic to identify *every* relevant obligation for proper deduplication + // of diagnostics. + errors.sort_by_key(|error| error.root_obligation.recursion_depth); for error in errors { + match ( + error.root_obligation.predicate.kind().skip_binder(), + error.obligation.predicate.kind().skip_binder(), + ) { + (ty::PredicateKind::Clause(ty::ClauseKind::Trait(predicate)), _) + if self.tcx.lang_items().index_trait() == Some(predicate.trait_ref.def_id) => + { + seen_preds.insert(error.obligation.predicate.kind().skip_binder()); + } + (_, ty::PredicateKind::Clause(ty::ClauseKind::Trait(predicate))) + if self.tcx.is_diagnostic_item(sym::SliceIndex, predicate.trait_ref.def_id) => + { + seen_preds.insert(error.obligation.predicate.kind().skip_binder()); + } + (root, pred) if seen_preds.contains(&pred) || seen_preds.contains(&root) => {} + _ => continue, + } error.obligation.cause.span = span; } } diff --git a/tests/ui/indexing/indexing-requires-a-uint.stderr b/tests/ui/indexing/indexing-requires-a-uint.stderr index 6ea6bb600e944..3041c2c99a108 100644 --- a/tests/ui/indexing/indexing-requires-a-uint.stderr +++ b/tests/ui/indexing/indexing-requires-a-uint.stderr @@ -8,6 +8,8 @@ LL | [0][0u8]; = help: the trait `SliceIndex<[{integer}]>` is implemented for `usize` = help: for that trait implementation, expected `usize`, found `u8` = note: required for `[{integer}]` to implement `Index` + = note: 1 redundant requirement hidden + = note: required for `[{integer}; 1]` to implement `Index` error[E0308]: mismatched types --> $DIR/indexing-requires-a-uint.rs:12:18 diff --git a/tests/ui/suggestions/suggest-dereferencing-index.stderr b/tests/ui/suggestions/suggest-dereferencing-index.stderr index adf01339972e5..23f6657f092a7 100644 --- a/tests/ui/suggestions/suggest-dereferencing-index.stderr +++ b/tests/ui/suggestions/suggest-dereferencing-index.stderr @@ -8,6 +8,8 @@ LL | let one_item_please: i32 = [1, 2, 3][i]; = help: the trait `SliceIndex<[{integer}]>` is implemented for `usize` = help: for that trait implementation, expected `usize`, found `&usize` = note: required for `[{integer}]` to implement `Index<&usize>` + = note: 1 redundant requirement hidden + = note: required for `[{integer}; 3]` to implement `Index<&usize>` help: dereference this index | LL | let one_item_please: i32 = [1, 2, 3][*i];