From 7af825fea4fea17d0b58fcd8c1f2b6fa11638cfb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 8 Jul 2024 16:23:01 -0400 Subject: [PATCH 1/3] Split out overflow handling into its own module --- .../src/error_reporting/traits/mod.rs | 2 + .../src/error_reporting/traits/overflow.rs | 197 ++++++++++++++++++ .../traits/type_err_ctxt_ext.rs | 182 +--------------- .../src/solve/normalize.rs | 2 +- .../src/traits/fulfill.rs | 2 +- .../src/traits/normalize.rs | 2 +- .../src/traits/query/normalize.rs | 2 +- .../src/traits/select/mod.rs | 2 +- compiler/rustc_traits/src/codegen.rs | 2 +- .../src/normalize_projection_ty.rs | 2 +- 10 files changed, 208 insertions(+), 187 deletions(-) create mode 100644 compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index 438b25056c338..7d334d623cc24 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -3,6 +3,7 @@ pub mod ambiguity; mod infer_ctxt_ext; pub mod on_unimplemented; +mod overflow; pub mod suggestions; mod type_err_ctxt_ext; @@ -17,6 +18,7 @@ use rustc_span::Span; use std::ops::ControlFlow; pub use self::infer_ctxt_ext::*; +pub use self::overflow::*; pub use self::type_err_ctxt_ext::*; // When outputting impl candidates, prefer showing those that are more similar. diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs new file mode 100644 index 0000000000000..061a5a4be2075 --- /dev/null +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/overflow.rs @@ -0,0 +1,197 @@ +use std::fmt; + +use rustc_errors::{ + struct_span_code_err, Diag, EmissionGuarantee, ErrorGuaranteed, FatalError, E0275, +}; +use rustc_hir::def::Namespace; +use rustc_hir::def_id::LOCAL_CRATE; +use rustc_infer::infer::error_reporting::TypeErrCtxt; +use rustc_infer::traits::{Obligation, PredicateObligation}; +use rustc_macros::extension; +use rustc_middle::ty::print::{FmtPrinter, Print}; +use rustc_middle::ty::{self, TyCtxt}; +use rustc_session::Limit; +use rustc_span::Span; +use rustc_type_ir::Upcast; + +use super::InferCtxtPrivExt; +use crate::error_reporting::traits::suggestions::TypeErrCtxtExt; + +pub enum OverflowCause<'tcx> { + DeeplyNormalize(ty::AliasTerm<'tcx>), + TraitSolver(ty::Predicate<'tcx>), +} + +pub fn suggest_new_overflow_limit<'tcx, G: EmissionGuarantee>( + tcx: TyCtxt<'tcx>, + err: &mut Diag<'_, G>, +) { + let suggested_limit = match tcx.recursion_limit() { + Limit(0) => Limit(2), + limit => limit * 2, + }; + err.help(format!( + "consider increasing the recursion limit by adding a \ + `#![recursion_limit = \"{}\"]` attribute to your crate (`{}`)", + suggested_limit, + tcx.crate_name(LOCAL_CRATE), + )); +} + +#[extension(pub trait TypeErrCtxtOverflowExt<'a, 'tcx>)] +impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { + /// Reports that an overflow has occurred and halts compilation. We + /// halt compilation unconditionally because it is important that + /// overflows never be masked -- they basically represent computations + /// whose result could not be truly determined and thus we can't say + /// if the program type checks or not -- and they are unusual + /// occurrences in any case. + fn report_overflow_error( + &self, + cause: OverflowCause<'tcx>, + span: Span, + suggest_increasing_limit: bool, + mutate: impl FnOnce(&mut Diag<'_>), + ) -> ! { + let mut err = self.build_overflow_error(cause, span, suggest_increasing_limit); + mutate(&mut err); + err.emit(); + FatalError.raise(); + } + + fn build_overflow_error( + &self, + cause: OverflowCause<'tcx>, + span: Span, + suggest_increasing_limit: bool, + ) -> Diag<'a> { + fn with_short_path<'tcx, T>(tcx: TyCtxt<'tcx>, value: T) -> String + where + T: fmt::Display + Print<'tcx, FmtPrinter<'tcx, 'tcx>>, + { + let s = value.to_string(); + if s.len() > 50 { + // We don't need to save the type to a file, we will be talking about this type already + // in a separate note when we explain the obligation, so it will be available that way. + let mut cx: FmtPrinter<'_, '_> = + FmtPrinter::new_with_limit(tcx, Namespace::TypeNS, rustc_session::Limit(6)); + value.print(&mut cx).unwrap(); + cx.into_buffer() + } else { + s + } + } + + let mut err = match cause { + OverflowCause::DeeplyNormalize(alias_term) => { + let alias_term = self.resolve_vars_if_possible(alias_term); + let kind = alias_term.kind(self.tcx).descr(); + let alias_str = with_short_path(self.tcx, alias_term); + struct_span_code_err!( + self.dcx(), + span, + E0275, + "overflow normalizing the {kind} `{alias_str}`", + ) + } + OverflowCause::TraitSolver(predicate) => { + let predicate = self.resolve_vars_if_possible(predicate); + match predicate.kind().skip_binder() { + ty::PredicateKind::Subtype(ty::SubtypePredicate { a, b, a_is_expected: _ }) + | ty::PredicateKind::Coerce(ty::CoercePredicate { a, b }) => { + struct_span_code_err!( + self.dcx(), + span, + E0275, + "overflow assigning `{a}` to `{b}`", + ) + } + _ => { + let pred_str = with_short_path(self.tcx, predicate); + struct_span_code_err!( + self.dcx(), + span, + E0275, + "overflow evaluating the requirement `{pred_str}`", + ) + } + } + } + }; + + if suggest_increasing_limit { + suggest_new_overflow_limit(self.tcx, &mut err); + } + + err + } + + /// Reports that an overflow has occurred and halts compilation. We + /// halt compilation unconditionally because it is important that + /// overflows never be masked -- they basically represent computations + /// whose result could not be truly determined and thus we can't say + /// if the program type checks or not -- and they are unusual + /// occurrences in any case. + fn report_overflow_obligation( + &self, + obligation: &Obligation<'tcx, T>, + suggest_increasing_limit: bool, + ) -> ! + where + T: Upcast, ty::Predicate<'tcx>> + Clone, + { + let predicate = obligation.predicate.clone().upcast(self.tcx); + let predicate = self.resolve_vars_if_possible(predicate); + self.report_overflow_error( + OverflowCause::TraitSolver(predicate), + obligation.cause.span, + suggest_increasing_limit, + |err| { + self.note_obligation_cause_code( + obligation.cause.body_id, + err, + predicate, + obligation.param_env, + obligation.cause.code(), + &mut vec![], + &mut Default::default(), + ); + }, + ); + } + + /// Reports that a cycle was detected which led to overflow and halts + /// compilation. This is equivalent to `report_overflow_obligation` except + /// that we can give a more helpful error message (and, in particular, + /// we do not suggest increasing the overflow limit, which is not + /// going to help). + fn report_overflow_obligation_cycle(&self, cycle: &[PredicateObligation<'tcx>]) -> ! { + let cycle = self.resolve_vars_if_possible(cycle.to_owned()); + assert!(!cycle.is_empty()); + + debug!(?cycle, "report_overflow_error_cycle"); + + // The 'deepest' obligation is most likely to have a useful + // cause 'backtrace' + self.report_overflow_obligation( + cycle.iter().max_by_key(|p| p.recursion_depth).unwrap(), + false, + ); + } + + fn report_overflow_no_abort( + &self, + obligation: PredicateObligation<'tcx>, + suggest_increasing_limit: bool, + ) -> ErrorGuaranteed { + let obligation = self.resolve_vars_if_possible(obligation); + let mut err = self.build_overflow_error( + OverflowCause::TraitSolver(obligation.predicate), + obligation.cause.span, + suggest_increasing_limit, + ); + self.note_obligation_cause(&mut err, &obligation); + self.point_at_returns_when_relevant(&mut err, &obligation); + err.emit() + } +} diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs index 7843a95d9667e..ee5f4f44251c0 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs @@ -3,6 +3,7 @@ use super::on_unimplemented::{AppendConstMessage, OnUnimplementedNote, TypeErrCtxtExt as _}; use super::suggestions::{get_explanation_based_on_obligation, TypeErrCtxtExt as _}; use crate::error_reporting::traits::infer_ctxt_ext::InferCtxtExt; +use crate::error_reporting::traits::overflow::TypeErrCtxtOverflowExt; use crate::error_reporting::traits::to_pretty_impl_header; use crate::error_reporting::traits::{ambiguity, ambiguity::CandidateSource::*}; use crate::errors::{ @@ -23,7 +24,7 @@ use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_data_structures::unord::UnordSet; use rustc_errors::codes::*; use rustc_errors::{pluralize, struct_span_code_err, Applicability, MultiSpan, StringPart}; -use rustc_errors::{Diag, EmissionGuarantee, ErrorGuaranteed, FatalError, StashKey}; +use rustc_errors::{Diag, ErrorGuaranteed, StashKey}; use rustc_hir::def::{DefKind, Namespace, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::Visitor; @@ -46,12 +47,9 @@ use rustc_middle::ty::{ TypeVisitableExt, Upcast, }; use rustc_middle::{bug, span_bug}; -use rustc_session::Limit; -use rustc_span::def_id::LOCAL_CRATE; use rustc_span::symbol::sym; use rustc_span::{BytePos, ExpnKind, Span, Symbol, DUMMY_SP}; use std::borrow::Cow; -use std::fmt; use std::iter; use super::{ @@ -61,27 +59,6 @@ use super::{ pub use rustc_infer::traits::error_reporting::*; -pub enum OverflowCause<'tcx> { - DeeplyNormalize(ty::AliasTerm<'tcx>), - TraitSolver(ty::Predicate<'tcx>), -} - -pub fn suggest_new_overflow_limit<'tcx, G: EmissionGuarantee>( - tcx: TyCtxt<'tcx>, - err: &mut Diag<'_, G>, -) { - let suggested_limit = match tcx.recursion_limit() { - Limit(0) => Limit(2), - limit => limit * 2, - }; - err.help(format!( - "consider increasing the recursion limit by adding a \ - `#![recursion_limit = \"{}\"]` attribute to your crate (`{}`)", - suggested_limit, - tcx.crate_name(LOCAL_CRATE), - )); -} - #[extension(pub trait TypeErrCtxtExt<'a, 'tcx>)] impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { fn report_fulfillment_errors( @@ -204,161 +181,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { reported.unwrap_or_else(|| self.dcx().delayed_bug("failed to report fulfillment errors")) } - /// Reports that an overflow has occurred and halts compilation. We - /// halt compilation unconditionally because it is important that - /// overflows never be masked -- they basically represent computations - /// whose result could not be truly determined and thus we can't say - /// if the program type checks or not -- and they are unusual - /// occurrences in any case. - fn report_overflow_error( - &self, - cause: OverflowCause<'tcx>, - span: Span, - suggest_increasing_limit: bool, - mutate: impl FnOnce(&mut Diag<'_>), - ) -> ! { - let mut err = self.build_overflow_error(cause, span, suggest_increasing_limit); - mutate(&mut err); - err.emit(); - FatalError.raise(); - } - - fn build_overflow_error( - &self, - cause: OverflowCause<'tcx>, - span: Span, - suggest_increasing_limit: bool, - ) -> Diag<'a> { - fn with_short_path<'tcx, T>(tcx: TyCtxt<'tcx>, value: T) -> String - where - T: fmt::Display + Print<'tcx, FmtPrinter<'tcx, 'tcx>>, - { - let s = value.to_string(); - if s.len() > 50 { - // We don't need to save the type to a file, we will be talking about this type already - // in a separate note when we explain the obligation, so it will be available that way. - let mut cx: FmtPrinter<'_, '_> = - FmtPrinter::new_with_limit(tcx, Namespace::TypeNS, rustc_session::Limit(6)); - value.print(&mut cx).unwrap(); - cx.into_buffer() - } else { - s - } - } - - let mut err = match cause { - OverflowCause::DeeplyNormalize(alias_term) => { - let alias_term = self.resolve_vars_if_possible(alias_term); - let kind = alias_term.kind(self.tcx).descr(); - let alias_str = with_short_path(self.tcx, alias_term); - struct_span_code_err!( - self.dcx(), - span, - E0275, - "overflow normalizing the {kind} `{alias_str}`", - ) - } - OverflowCause::TraitSolver(predicate) => { - let predicate = self.resolve_vars_if_possible(predicate); - match predicate.kind().skip_binder() { - ty::PredicateKind::Subtype(ty::SubtypePredicate { a, b, a_is_expected: _ }) - | ty::PredicateKind::Coerce(ty::CoercePredicate { a, b }) => { - struct_span_code_err!( - self.dcx(), - span, - E0275, - "overflow assigning `{a}` to `{b}`", - ) - } - _ => { - let pred_str = with_short_path(self.tcx, predicate); - struct_span_code_err!( - self.dcx(), - span, - E0275, - "overflow evaluating the requirement `{pred_str}`", - ) - } - } - } - }; - - if suggest_increasing_limit { - suggest_new_overflow_limit(self.tcx, &mut err); - } - - err - } - - /// Reports that an overflow has occurred and halts compilation. We - /// halt compilation unconditionally because it is important that - /// overflows never be masked -- they basically represent computations - /// whose result could not be truly determined and thus we can't say - /// if the program type checks or not -- and they are unusual - /// occurrences in any case. - fn report_overflow_obligation( - &self, - obligation: &Obligation<'tcx, T>, - suggest_increasing_limit: bool, - ) -> ! - where - T: Upcast, ty::Predicate<'tcx>> + Clone, - { - let predicate = obligation.predicate.clone().upcast(self.tcx); - let predicate = self.resolve_vars_if_possible(predicate); - self.report_overflow_error( - OverflowCause::TraitSolver(predicate), - obligation.cause.span, - suggest_increasing_limit, - |err| { - self.note_obligation_cause_code( - obligation.cause.body_id, - err, - predicate, - obligation.param_env, - obligation.cause.code(), - &mut vec![], - &mut Default::default(), - ); - }, - ); - } - - /// Reports that a cycle was detected which led to overflow and halts - /// compilation. This is equivalent to `report_overflow_obligation` except - /// that we can give a more helpful error message (and, in particular, - /// we do not suggest increasing the overflow limit, which is not - /// going to help). - fn report_overflow_obligation_cycle(&self, cycle: &[PredicateObligation<'tcx>]) -> ! { - let cycle = self.resolve_vars_if_possible(cycle.to_owned()); - assert!(!cycle.is_empty()); - - debug!(?cycle, "report_overflow_error_cycle"); - - // The 'deepest' obligation is most likely to have a useful - // cause 'backtrace' - self.report_overflow_obligation( - cycle.iter().max_by_key(|p| p.recursion_depth).unwrap(), - false, - ); - } - - fn report_overflow_no_abort( - &self, - obligation: PredicateObligation<'tcx>, - suggest_increasing_limit: bool, - ) -> ErrorGuaranteed { - let obligation = self.resolve_vars_if_possible(obligation); - let mut err = self.build_overflow_error( - OverflowCause::TraitSolver(obligation.predicate), - obligation.cause.span, - suggest_increasing_limit, - ); - self.note_obligation_cause(&mut err, &obligation); - self.point_at_returns_when_relevant(&mut err, &obligation); - err.emit() - } - /// The `root_obligation` parameter should be the `root_obligation` field /// from a `FulfillmentError`. If no `FulfillmentError` is available, /// then it should be the same as `obligation`. diff --git a/compiler/rustc_trait_selection/src/solve/normalize.rs b/compiler/rustc_trait_selection/src/solve/normalize.rs index bda2137fa7368..ca313590265d2 100644 --- a/compiler/rustc_trait_selection/src/solve/normalize.rs +++ b/compiler/rustc_trait_selection/src/solve/normalize.rs @@ -1,7 +1,7 @@ use std::fmt::Debug; use std::marker::PhantomData; -use crate::error_reporting::traits::{OverflowCause, TypeErrCtxtExt}; +use crate::error_reporting::traits::{OverflowCause, TypeErrCtxtOverflowExt}; use crate::traits::query::evaluate_obligation::InferCtxtExt; use crate::traits::{BoundVarReplacer, PlaceholderReplacer, ScrubbedTraitError}; use rustc_data_structures::stack::ensure_sufficient_stack; diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs index 6f7e69798ee92..5597c8be59258 100644 --- a/compiler/rustc_trait_selection/src/traits/fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs @@ -1,4 +1,4 @@ -use crate::error_reporting::traits::TypeErrCtxtExt; +use crate::error_reporting::traits::TypeErrCtxtOverflowExt; use crate::infer::{InferCtxt, TyOrConstInferVar}; use crate::traits::normalize::normalize_with_depth_to; use rustc_data_structures::captures::Captures; diff --git a/compiler/rustc_trait_selection/src/traits/normalize.rs b/compiler/rustc_trait_selection/src/traits/normalize.rs index a34fd80446785..dda3aaaf71e82 100644 --- a/compiler/rustc_trait_selection/src/traits/normalize.rs +++ b/compiler/rustc_trait_selection/src/traits/normalize.rs @@ -3,7 +3,7 @@ use super::SelectionContext; use super::{project, with_replaced_escaping_bound_vars, BoundVarReplacer, PlaceholderReplacer}; use crate::error_reporting::traits::OverflowCause; -use crate::error_reporting::traits::TypeErrCtxtExt; +use crate::error_reporting::traits::TypeErrCtxtOverflowExt; use crate::solve::NextSolverError; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_infer::infer::at::At; diff --git a/compiler/rustc_trait_selection/src/traits/query/normalize.rs b/compiler/rustc_trait_selection/src/traits/query/normalize.rs index da4e4fef3cf78..c11e86abef8a4 100644 --- a/compiler/rustc_trait_selection/src/traits/query/normalize.rs +++ b/compiler/rustc_trait_selection/src/traits/query/normalize.rs @@ -3,7 +3,7 @@ //! `normalize_canonicalized_projection_ty` query when it encounters projections. use crate::error_reporting::traits::OverflowCause; -use crate::error_reporting::traits::TypeErrCtxtExt; +use crate::error_reporting::traits::TypeErrCtxtOverflowExt; use crate::infer::at::At; use crate::infer::canonical::OriginalQueryValues; use crate::infer::{InferCtxt, InferOk}; diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 1e55b2d015003..2c4ffdabf140e 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -18,7 +18,7 @@ use super::{ TraitQueryMode, }; -use crate::error_reporting::traits::TypeErrCtxtExt; +use crate::error_reporting::traits::TypeErrCtxtOverflowExt; use crate::infer::{InferCtxt, InferCtxtExt, InferOk, TypeFreshener}; use crate::solve::InferCtxtSelectExt as _; use crate::traits::normalize::normalize_with_depth; diff --git a/compiler/rustc_traits/src/codegen.rs b/compiler/rustc_traits/src/codegen.rs index 51ac7b2c7ac1b..3ee5fd876ff57 100644 --- a/compiler/rustc_traits/src/codegen.rs +++ b/compiler/rustc_traits/src/codegen.rs @@ -7,7 +7,7 @@ use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::bug; use rustc_middle::traits::CodegenObligationError; use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt}; -use rustc_trait_selection::error_reporting::traits::TypeErrCtxtExt; +use rustc_trait_selection::error_reporting::traits::TypeErrCtxtOverflowExt; use rustc_trait_selection::traits::{ ImplSource, Obligation, ObligationCause, ObligationCtxt, ScrubbedTraitError, SelectionContext, Unimplemented, diff --git a/compiler/rustc_traits/src/normalize_projection_ty.rs b/compiler/rustc_traits/src/normalize_projection_ty.rs index 85d84983d28fa..2d70fdc3935fb 100644 --- a/compiler/rustc_traits/src/normalize_projection_ty.rs +++ b/compiler/rustc_traits/src/normalize_projection_ty.rs @@ -2,7 +2,7 @@ use rustc_infer::infer::canonical::{Canonical, QueryResponse}; use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::query::Providers; use rustc_middle::ty::{ParamEnvAnd, TyCtxt}; -use rustc_trait_selection::error_reporting::traits::TypeErrCtxtExt; +use rustc_trait_selection::error_reporting::traits::TypeErrCtxtOverflowExt; use rustc_trait_selection::infer::InferCtxtBuilderExt; use rustc_trait_selection::traits::query::{ normalize::NormalizationResult, CanonicalAliasGoal, NoSolution, From cd68a28daa460e04875a16b9677117af93046979 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 8 Jul 2024 16:44:00 -0400 Subject: [PATCH 2/3] Move some stuff into the ambiguity and suggestion modules --- .../src/error_reporting/traits/ambiguity.rs | 568 ++++++++++++++- .../src/error_reporting/traits/mod.rs | 58 -- .../src/error_reporting/traits/suggestions.rs | 169 +++++ .../traits/type_err_ctxt_ext.rs | 681 +----------------- 4 files changed, 742 insertions(+), 734 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs index e6cb28df593af..c301deac6164d 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs @@ -1,10 +1,27 @@ +use std::ops::ControlFlow; + +use rustc_errors::{ + struct_span_code_err, Applicability, Diag, MultiSpan, StashKey, E0283, E0284, E0790, +}; +use rustc_hir as hir; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; +use rustc_hir::intravisit::Visitor as _; +use rustc_hir::LangItem; +use rustc_infer::infer::error_reporting::{TypeAnnotationNeeded, TypeErrCtxt}; use rustc_infer::infer::{BoundRegionConversionTime, InferCtxt}; use rustc_infer::traits::util::elaborate; -use rustc_infer::traits::{Obligation, ObligationCause, PolyTraitObligation}; -use rustc_middle::ty; -use rustc_span::{Span, DUMMY_SP}; +use rustc_infer::traits::{ + Obligation, ObligationCause, ObligationCauseCode, PolyTraitObligation, PredicateObligation, +}; +use rustc_macros::extension; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitable as _, TypeVisitableExt as _}; +use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP}; +use crate::error_reporting::traits::suggestions::TypeErrCtxtExt as _; +use crate::error_reporting::traits::{ + to_pretty_impl_header, FindExprBySpan, InferCtxtPrivExt as _, +}; use crate::traits::query::evaluate_obligation::InferCtxtExt; use crate::traits::ObligationCtxt; @@ -134,3 +151,548 @@ pub fn compute_applicable_impls_for_diagnostics<'tcx>( ambiguities } + +#[extension(pub trait TypeErrCtxtAmbiguityExt<'a, 'tcx>)] +impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { + #[instrument(skip(self), level = "debug")] + fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>) -> ErrorGuaranteed { + // Unable to successfully determine, probably means + // insufficient type information, but could mean + // ambiguous impls. The latter *ought* to be a + // coherence violation, so we don't report it here. + + let predicate = self.resolve_vars_if_possible(obligation.predicate); + let span = obligation.cause.span; + + debug!(?predicate, obligation.cause.code = ?obligation.cause.code()); + + // Ambiguity errors are often caused as fallout from earlier errors. + // We ignore them if this `infcx` is tainted in some cases below. + + let bound_predicate = predicate.kind(); + let mut err = match bound_predicate.skip_binder() { + ty::PredicateKind::Clause(ty::ClauseKind::Trait(data)) => { + let trait_ref = bound_predicate.rebind(data.trait_ref); + debug!(?trait_ref); + + if let Err(e) = predicate.error_reported() { + return e; + } + + if let Err(guar) = self.tcx.ensure().coherent_trait(trait_ref.def_id()) { + // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case + // other `Foo` impls are incoherent. + return guar; + } + + // This is kind of a hack: it frequently happens that some earlier + // error prevents types from being fully inferred, and then we get + // a bunch of uninteresting errors saying something like " doesn't implement Sized". It may even be true that we + // could just skip over all checks where the self-ty is an + // inference variable, but I was afraid that there might be an + // inference variable created, registered as an obligation, and + // then never forced by writeback, and hence by skipping here we'd + // be ignoring the fact that we don't KNOW the type works + // out. Though even that would probably be harmless, given that + // we're only talking about builtin traits, which are known to be + // inhabited. We used to check for `self.tcx.sess.has_errors()` to + // avoid inundating the user with unnecessary errors, but we now + // check upstream for type errors and don't add the obligations to + // begin with in those cases. + if self.tcx.is_lang_item(trait_ref.def_id(), LangItem::Sized) { + match self.tainted_by_errors() { + None => { + let err = self.emit_inference_failure_err( + obligation.cause.body_id, + span, + trait_ref.self_ty().skip_binder().into(), + TypeAnnotationNeeded::E0282, + false, + ); + return err.stash(span, StashKey::MaybeForgetReturn).unwrap(); + } + Some(e) => return e, + } + } + + // Typically, this ambiguity should only happen if + // there are unresolved type inference variables + // (otherwise it would suggest a coherence + // failure). But given #21974 that is not necessarily + // the case -- we can have multiple where clauses that + // are only distinguished by a region, which results + // in an ambiguity even when all types are fully + // known, since we don't dispatch based on region + // relationships. + + // Pick the first generic parameter that still contains inference variables as the one + // we're going to emit an error for. If there are none (see above), fall back to + // a more general error. + let arg = data.trait_ref.args.iter().find(|s| s.has_non_region_infer()); + + let mut err = if let Some(arg) = arg { + self.emit_inference_failure_err( + obligation.cause.body_id, + span, + arg, + TypeAnnotationNeeded::E0283, + true, + ) + } else { + struct_span_code_err!( + self.dcx(), + span, + E0283, + "type annotations needed: cannot satisfy `{}`", + predicate, + ) + }; + + let mut ambiguities = compute_applicable_impls_for_diagnostics( + self.infcx, + &obligation.with(self.tcx, trait_ref), + ); + let has_non_region_infer = + trait_ref.skip_binder().args.types().any(|t| !t.is_ty_or_numeric_infer()); + // It doesn't make sense to talk about applicable impls if there are more than a + // handful of them. If there are a lot of them, but only a few of them have no type + // params, we only show those, as they are more likely to be useful/intended. + if ambiguities.len() > 5 { + let infcx = self.infcx; + if !ambiguities.iter().all(|option| match option { + CandidateSource::DefId(did) => infcx.tcx.generics_of(*did).count() == 0, + CandidateSource::ParamEnv(_) => true, + }) { + // If not all are blanket impls, we filter blanked impls out. + ambiguities.retain(|option| match option { + CandidateSource::DefId(did) => infcx.tcx.generics_of(*did).count() == 0, + CandidateSource::ParamEnv(_) => true, + }); + } + } + if ambiguities.len() > 1 && ambiguities.len() < 10 && has_non_region_infer { + if let Some(e) = self.tainted_by_errors() + && arg.is_none() + { + // If `arg.is_none()`, then this is probably two param-env + // candidates or impl candidates that are equal modulo lifetimes. + // Therefore, if we've already emitted an error, just skip this + // one, since it's not particularly actionable. + err.cancel(); + return e; + } + self.annotate_source_of_ambiguity(&mut err, &ambiguities, predicate); + } else { + if let Some(e) = self.tainted_by_errors() { + err.cancel(); + return e; + } + err.note(format!("cannot satisfy `{predicate}`")); + let impl_candidates = + self.find_similar_impl_candidates(predicate.as_trait_clause().unwrap()); + if impl_candidates.len() < 40 { + self.report_similar_impl_candidates( + impl_candidates.as_slice(), + trait_ref, + obligation.cause.body_id, + &mut err, + false, + obligation.param_env, + ); + } + } + + if let ObligationCauseCode::WhereClause(def_id, _) + | ObligationCauseCode::WhereClauseInExpr(def_id, ..) = *obligation.cause.code() + { + self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id()); + } + + if let Some(ty::GenericArgKind::Type(_)) = arg.map(|arg| arg.unpack()) + && let Some(body) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) + { + let mut expr_finder = FindExprBySpan::new(span, self.tcx); + expr_finder.visit_expr(&body.value); + + if let Some(hir::Expr { + kind: + hir::ExprKind::Call( + hir::Expr { + kind: hir::ExprKind::Path(hir::QPath::Resolved(None, path)), + .. + }, + _, + ) + | hir::ExprKind::Path(hir::QPath::Resolved(None, path)), + .. + }) = expr_finder.result + && let [ + .., + trait_path_segment @ hir::PathSegment { + res: Res::Def(DefKind::Trait, trait_id), + .. + }, + hir::PathSegment { + ident: assoc_item_name, + res: Res::Def(_, item_id), + .. + }, + ] = path.segments + && data.trait_ref.def_id == *trait_id + && self.tcx.trait_of_item(*item_id) == Some(*trait_id) + && let None = self.tainted_by_errors() + { + let (verb, noun) = match self.tcx.associated_item(item_id).kind { + ty::AssocKind::Const => ("refer to the", "constant"), + ty::AssocKind::Fn => ("call", "function"), + // This is already covered by E0223, but this following single match + // arm doesn't hurt here. + ty::AssocKind::Type => ("refer to the", "type"), + }; + + // Replace the more general E0283 with a more specific error + err.cancel(); + err = self.dcx().struct_span_err( + span, + format!( + "cannot {verb} associated {noun} on trait without specifying the \ + corresponding `impl` type", + ), + ); + err.code(E0790); + + if let Some(local_def_id) = data.trait_ref.def_id.as_local() + && let hir::Node::Item(hir::Item { + ident: trait_name, + kind: hir::ItemKind::Trait(_, _, _, _, trait_item_refs), + .. + }) = self.tcx.hir_node_by_def_id(local_def_id) + && let Some(method_ref) = trait_item_refs + .iter() + .find(|item_ref| item_ref.ident == *assoc_item_name) + { + err.span_label( + method_ref.span, + format!("`{trait_name}::{assoc_item_name}` defined here"), + ); + } + + err.span_label(span, format!("cannot {verb} associated {noun} of trait")); + + let trait_impls = self.tcx.trait_impls_of(data.trait_ref.def_id); + + if let Some(impl_def_id) = + trait_impls.non_blanket_impls().values().flatten().next() + { + let non_blanket_impl_count = + trait_impls.non_blanket_impls().values().flatten().count(); + // If there is only one implementation of the trait, suggest using it. + // Otherwise, use a placeholder comment for the implementation. + let (message, self_type) = if non_blanket_impl_count == 1 { + ( + "use the fully-qualified path to the only available \ + implementation", + format!( + "{}", + self.tcx.type_of(impl_def_id).instantiate_identity() + ), + ) + } else { + ( + "use a fully-qualified path to a specific available \ + implementation", + "/* self type */".to_string(), + ) + }; + let mut suggestions = + vec![(path.span.shrink_to_lo(), format!("<{self_type} as "))]; + if let Some(generic_arg) = trait_path_segment.args { + let between_span = + trait_path_segment.ident.span.between(generic_arg.span_ext); + // get rid of :: between Trait and + // must be '::' between them, otherwise the parser won't accept the code + suggestions.push((between_span, "".to_string())); + suggestions + .push((generic_arg.span_ext.shrink_to_hi(), ">".to_string())); + } else { + suggestions.push(( + trait_path_segment.ident.span.shrink_to_hi(), + ">".to_string(), + )); + } + err.multipart_suggestion( + message, + suggestions, + Applicability::MaybeIncorrect, + ); + } + } + }; + + err + } + + ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(arg)) => { + // Same hacky approach as above to avoid deluging user + // with error messages. + + if let Err(e) = arg.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; + } + + self.emit_inference_failure_err( + obligation.cause.body_id, + span, + arg, + TypeAnnotationNeeded::E0282, + false, + ) + } + + ty::PredicateKind::Subtype(data) => { + if let Err(e) = data.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; + } + let ty::SubtypePredicate { a_is_expected: _, a, b } = data; + // both must be type variables, or the other would've been instantiated + assert!(a.is_ty_var() && b.is_ty_var()); + self.emit_inference_failure_err( + obligation.cause.body_id, + span, + a.into(), + TypeAnnotationNeeded::E0282, + true, + ) + } + ty::PredicateKind::Clause(ty::ClauseKind::Projection(data)) => { + if let Err(e) = predicate.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; + } + + if let Err(guar) = + self.tcx.ensure().coherent_trait(self.tcx.parent(data.projection_term.def_id)) + { + // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case + // other `Foo` impls are incoherent. + return guar; + } + let arg = data + .projection_term + .args + .iter() + .chain(Some(data.term.into_arg())) + .find(|g| g.has_non_region_infer()); + if let Some(arg) = arg { + self.emit_inference_failure_err( + obligation.cause.body_id, + span, + arg, + TypeAnnotationNeeded::E0284, + true, + ) + .with_note(format!("cannot satisfy `{predicate}`")) + } else { + // If we can't find a generic parameter, just print a generic error + struct_span_code_err!( + self.dcx(), + span, + E0284, + "type annotations needed: cannot satisfy `{}`", + predicate, + ) + .with_span_label(span, format!("cannot satisfy `{predicate}`")) + } + } + + ty::PredicateKind::Clause(ty::ClauseKind::ConstEvaluatable(data)) => { + if let Err(e) = predicate.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; + } + let arg = data.walk().find(|g| g.is_non_region_infer()); + if let Some(arg) = arg { + let err = self.emit_inference_failure_err( + obligation.cause.body_id, + span, + arg, + TypeAnnotationNeeded::E0284, + true, + ); + err + } else { + // If we can't find a generic parameter, just print a generic error + struct_span_code_err!( + self.dcx(), + span, + E0284, + "type annotations needed: cannot satisfy `{}`", + predicate, + ) + .with_span_label(span, format!("cannot satisfy `{predicate}`")) + } + } + + ty::PredicateKind::Clause(ty::ClauseKind::ConstArgHasType(ct, ..)) => self + .emit_inference_failure_err( + obligation.cause.body_id, + span, + ct.into(), + TypeAnnotationNeeded::E0284, + true, + ), + ty::PredicateKind::NormalizesTo(ty::NormalizesTo { alias, term }) + if term.is_infer() => + { + if let Some(e) = self.tainted_by_errors() { + return e; + } + struct_span_code_err!( + self.dcx(), + span, + E0284, + "type annotations needed: cannot normalize `{alias}`", + ) + .with_span_label(span, format!("cannot normalize `{alias}`")) + } + + _ => { + if let Some(e) = self.tainted_by_errors() { + return e; + } + struct_span_code_err!( + self.dcx(), + span, + E0284, + "type annotations needed: cannot satisfy `{}`", + predicate, + ) + .with_span_label(span, format!("cannot satisfy `{predicate}`")) + } + }; + self.note_obligation_cause(&mut err, obligation); + err.emit() + } + + fn annotate_source_of_ambiguity( + &self, + err: &mut Diag<'_>, + ambiguities: &[CandidateSource], + predicate: ty::Predicate<'tcx>, + ) { + let mut spans = vec![]; + let mut crates = vec![]; + let mut post = vec![]; + let mut has_param_env = false; + for ambiguity in ambiguities { + match ambiguity { + CandidateSource::DefId(impl_def_id) => match self.tcx.span_of_impl(*impl_def_id) { + Ok(span) => spans.push(span), + Err(name) => { + crates.push(name); + if let Some(header) = to_pretty_impl_header(self.tcx, *impl_def_id) { + post.push(header); + } + } + }, + CandidateSource::ParamEnv(span) => { + has_param_env = true; + spans.push(*span); + } + } + } + let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{n}`")).collect(); + crate_names.sort(); + crate_names.dedup(); + post.sort(); + post.dedup(); + + if self.tainted_by_errors().is_some() + && (crate_names.len() == 1 + && spans.len() == 0 + && ["`core`", "`alloc`", "`std`"].contains(&crate_names[0].as_str()) + || predicate.visit_with(&mut HasNumericInferVisitor).is_break()) + { + // Avoid complaining about other inference issues for expressions like + // `42 >> 1`, where the types are still `{integer}`, but we want to + // Do we need `trait_ref.skip_binder().self_ty().is_numeric() &&` too? + // NOTE(eddyb) this was `.cancel()`, but `err` + // is borrowed, so we can't fully defuse it. + err.downgrade_to_delayed_bug(); + return; + } + + let msg = format!( + "multiple `impl`s{} satisfying `{}` found", + if has_param_env { " or `where` clauses" } else { "" }, + predicate + ); + let post = if post.len() > 1 || (post.len() == 1 && post[0].contains('\n')) { + format!(":\n{}", post.iter().map(|p| format!("- {p}")).collect::>().join("\n"),) + } else if post.len() == 1 { + format!(": `{}`", post[0]) + } else { + String::new() + }; + + match (spans.len(), crates.len(), crate_names.len()) { + (0, 0, 0) => { + err.note(format!("cannot satisfy `{predicate}`")); + } + (0, _, 1) => { + err.note(format!("{} in the `{}` crate{}", msg, crates[0], post,)); + } + (0, _, _) => { + err.note(format!( + "{} in the following crates: {}{}", + msg, + crate_names.join(", "), + post, + )); + } + (_, 0, 0) => { + let span: MultiSpan = spans.into(); + err.span_note(span, msg); + } + (_, 1, 1) => { + let span: MultiSpan = spans.into(); + err.span_note(span, msg); + err.note(format!("and another `impl` found in the `{}` crate{}", crates[0], post,)); + } + _ => { + let span: MultiSpan = spans.into(); + err.span_note(span, msg); + err.note(format!( + "and more `impl`s found in the following crates: {}{}", + crate_names.join(", "), + post, + )); + } + } + } +} + +struct HasNumericInferVisitor; + +impl<'tcx> ty::TypeVisitor> for HasNumericInferVisitor { + type Result = ControlFlow<()>; + + fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { + if matches!(ty.kind(), ty::Infer(ty::FloatVar(_) | ty::IntVar(_))) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) + } + } +} diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index 7d334d623cc24..e776248e68476 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -15,7 +15,6 @@ use rustc_infer::traits::{Obligation, ObligationCause, ObligationCauseCode, Pred use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::Span; -use std::ops::ControlFlow; pub use self::infer_ctxt_ext::*; pub use self::overflow::*; @@ -91,49 +90,6 @@ impl<'v> Visitor<'v> for FindExprBySpan<'v> { } } -/// Look for type `param` in an ADT being used only through a reference to confirm that suggesting -/// `param: ?Sized` would be a valid constraint. -struct FindTypeParam { - param: rustc_span::Symbol, - invalid_spans: Vec, - nested: bool, -} - -impl<'v> Visitor<'v> for FindTypeParam { - fn visit_where_predicate(&mut self, _: &'v hir::WherePredicate<'v>) { - // Skip where-clauses, to avoid suggesting indirection for type parameters found there. - } - - fn visit_ty(&mut self, ty: &hir::Ty<'_>) { - // We collect the spans of all uses of the "bare" type param, like in `field: T` or - // `field: (T, T)` where we could make `T: ?Sized` while skipping cases that are known to be - // valid like `field: &'a T` or `field: *mut T` and cases that *might* have further `Sized` - // obligations like `Box` and `Vec`, but we perform no extra analysis for those cases - // and suggest `T: ?Sized` regardless of their obligations. This is fine because the errors - // in that case should make what happened clear enough. - match ty.kind { - hir::TyKind::Ptr(_) | hir::TyKind::Ref(..) | hir::TyKind::TraitObject(..) => {} - hir::TyKind::Path(hir::QPath::Resolved(None, path)) - if path.segments.len() == 1 && path.segments[0].ident.name == self.param => - { - if !self.nested { - debug!(?ty, "FindTypeParam::visit_ty"); - self.invalid_spans.push(ty.span); - } - } - hir::TyKind::Path(_) => { - let prev = self.nested; - self.nested = true; - hir::intravisit::walk_ty(self, ty); - self.nested = prev; - } - _ => { - hir::intravisit::walk_ty(self, ty); - } - } - } -} - /// Summarizes information #[derive(Clone)] pub enum ArgKind { @@ -165,20 +121,6 @@ impl ArgKind { } } -struct HasNumericInferVisitor; - -impl<'tcx> ty::TypeVisitor> for HasNumericInferVisitor { - type Result = ControlFlow<()>; - - fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result { - if matches!(ty.kind(), ty::Infer(ty::FloatVar(_) | ty::IntVar(_))) { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } - } -} - #[derive(Copy, Clone)] pub enum DefIdOrName { DefId(DefId), diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index fdc6db4abec0b..11a2b1d323379 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -4623,6 +4623,132 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { ); } } + + #[instrument(level = "debug", skip_all)] + fn suggest_unsized_bound_if_applicable( + &self, + err: &mut Diag<'_>, + obligation: &PredicateObligation<'tcx>, + ) { + let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) = + obligation.predicate.kind().skip_binder() + else { + return; + }; + let (ObligationCauseCode::WhereClause(item_def_id, span) + | ObligationCauseCode::WhereClauseInExpr(item_def_id, span, ..)) = + *obligation.cause.code().peel_derives() + else { + return; + }; + if span.is_dummy() { + return; + } + debug!(?pred, ?item_def_id, ?span); + + let (Some(node), true) = ( + self.tcx.hir().get_if_local(item_def_id), + self.tcx.is_lang_item(pred.def_id(), LangItem::Sized), + ) else { + return; + }; + + let Some(generics) = node.generics() else { + return; + }; + let sized_trait = self.tcx.lang_items().sized_trait(); + debug!(?generics.params); + debug!(?generics.predicates); + let Some(param) = generics.params.iter().find(|param| param.span == span) else { + return; + }; + // Check that none of the explicit trait bounds is `Sized`. Assume that an explicit + // `Sized` bound is there intentionally and we don't need to suggest relaxing it. + let explicitly_sized = generics + .bounds_for_param(param.def_id) + .flat_map(|bp| bp.bounds) + .any(|bound| bound.trait_ref().and_then(|tr| tr.trait_def_id()) == sized_trait); + if explicitly_sized { + return; + } + debug!(?param); + match node { + hir::Node::Item( + item @ hir::Item { + // Only suggest indirection for uses of type parameters in ADTs. + kind: + hir::ItemKind::Enum(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Union(..), + .. + }, + ) => { + if self.suggest_indirection_for_unsized(err, item, param) { + return; + } + } + _ => {} + }; + + // Didn't add an indirection suggestion, so add a general suggestion to relax `Sized`. + let (span, separator, open_paren_sp) = + if let Some((s, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) { + (s, " +", open_paren_sp) + } else { + (param.name.ident().span.shrink_to_hi(), ":", None) + }; + + let mut suggs = vec![]; + let suggestion = format!("{separator} ?Sized"); + + if let Some(open_paren_sp) = open_paren_sp { + suggs.push((open_paren_sp, "(".to_string())); + suggs.push((span, format!("){suggestion}"))); + } else { + suggs.push((span, suggestion)); + } + + err.multipart_suggestion_verbose( + "consider relaxing the implicit `Sized` restriction", + suggs, + Applicability::MachineApplicable, + ); + } + + fn suggest_indirection_for_unsized( + &self, + err: &mut Diag<'_>, + item: &hir::Item<'tcx>, + param: &hir::GenericParam<'tcx>, + ) -> bool { + // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a + // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S(T);` + // is not. Look for invalid "bare" parameter uses, and suggest using indirection. + let mut visitor = + FindTypeParam { param: param.name.ident().name, invalid_spans: vec![], nested: false }; + visitor.visit_item(item); + if visitor.invalid_spans.is_empty() { + return false; + } + let mut multispan: MultiSpan = param.span.into(); + multispan.push_span_label( + param.span, + format!("this could be changed to `{}: ?Sized`...", param.name.ident()), + ); + for sp in visitor.invalid_spans { + multispan.push_span_label( + sp, + format!("...if indirection were used here: `Box<{}>`", param.name.ident()), + ); + } + err.span_help( + multispan, + format!( + "you could relax the implicit `Sized` bound on `{T}` if it were \ + used through indirection like `&{T}` or `Box<{T}>`", + T = param.name.ident(), + ), + ); + true + } } /// Add a hint to add a missing borrow or remove an unnecessary one. @@ -5126,3 +5252,46 @@ fn get_deref_type_and_refs(mut ty: Ty<'_>) -> (Ty<'_>, Vec) { (ty, refs) } + +/// Look for type `param` in an ADT being used only through a reference to confirm that suggesting +/// `param: ?Sized` would be a valid constraint. +struct FindTypeParam { + param: rustc_span::Symbol, + invalid_spans: Vec, + nested: bool, +} + +impl<'v> Visitor<'v> for FindTypeParam { + fn visit_where_predicate(&mut self, _: &'v hir::WherePredicate<'v>) { + // Skip where-clauses, to avoid suggesting indirection for type parameters found there. + } + + fn visit_ty(&mut self, ty: &hir::Ty<'_>) { + // We collect the spans of all uses of the "bare" type param, like in `field: T` or + // `field: (T, T)` where we could make `T: ?Sized` while skipping cases that are known to be + // valid like `field: &'a T` or `field: *mut T` and cases that *might* have further `Sized` + // obligations like `Box` and `Vec`, but we perform no extra analysis for those cases + // and suggest `T: ?Sized` regardless of their obligations. This is fine because the errors + // in that case should make what happened clear enough. + match ty.kind { + hir::TyKind::Ptr(_) | hir::TyKind::Ref(..) | hir::TyKind::TraitObject(..) => {} + hir::TyKind::Path(hir::QPath::Resolved(None, path)) + if path.segments.len() == 1 && path.segments[0].ident.name == self.param => + { + if !self.nested { + debug!(?ty, "FindTypeParam::visit_ty"); + self.invalid_spans.push(ty.span); + } + } + hir::TyKind::Path(_) => { + let prev = self.nested; + self.nested = true; + hir::intravisit::walk_ty(self, ty); + self.nested = prev; + } + _ => { + hir::intravisit::walk_ty(self, ty); + } + } + } +} diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs index ee5f4f44251c0..dec3b26420c24 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs @@ -1,15 +1,12 @@ -// ignore-tidy-filelength :( - +use super::ambiguity::TypeErrCtxtAmbiguityExt as _; use super::on_unimplemented::{AppendConstMessage, OnUnimplementedNote, TypeErrCtxtExt as _}; use super::suggestions::{get_explanation_based_on_obligation, TypeErrCtxtExt as _}; use crate::error_reporting::traits::infer_ctxt_ext::InferCtxtExt; use crate::error_reporting::traits::overflow::TypeErrCtxtOverflowExt; -use crate::error_reporting::traits::to_pretty_impl_header; -use crate::error_reporting::traits::{ambiguity, ambiguity::CandidateSource::*}; use crate::errors::{ AsyncClosureNotFn, ClosureFnMutLabel, ClosureFnOnceLabel, ClosureKindMismatch, }; -use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode}; +use crate::infer::error_reporting::TyCategory; use crate::infer::InferCtxtExt as _; use crate::infer::{self, InferCtxt}; use crate::traits::query::evaluate_obligation::InferCtxtExt as _; @@ -23,13 +20,13 @@ use core::ops::ControlFlow; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_data_structures::unord::UnordSet; use rustc_errors::codes::*; -use rustc_errors::{pluralize, struct_span_code_err, Applicability, MultiSpan, StringPart}; +use rustc_errors::{pluralize, struct_span_code_err, Applicability, StringPart}; use rustc_errors::{Diag, ErrorGuaranteed, StashKey}; -use rustc_hir::def::{DefKind, Namespace, Res}; +use rustc_hir::def::Namespace; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::Visitor; +use rustc_hir::Node; use rustc_hir::{self as hir, LangItem}; -use rustc_hir::{GenericParam, Item, Node}; use rustc_infer::infer::error_reporting::TypeErrCtxt; use rustc_infer::infer::{InferOk, TypeTrace}; use rustc_macros::extension; @@ -43,8 +40,7 @@ use rustc_middle::ty::print::{ PrintTraitRefExt as _, }; use rustc_middle::ty::{ - self, SubtypePredicate, ToPolyTraitRef, TraitRef, Ty, TyCtxt, TypeFoldable, TypeVisitable, - TypeVisitableExt, Upcast, + self, ToPolyTraitRef, TraitRef, Ty, TyCtxt, TypeFoldable, TypeVisitableExt, Upcast, }; use rustc_middle::{bug, span_bug}; use rustc_span::symbol::sym; @@ -53,8 +49,7 @@ use std::borrow::Cow; use std::iter; use super::{ - ArgKind, CandidateSimilarity, FindExprBySpan, FindTypeParam, GetSafeTransmuteErrorAndReason, - HasNumericInferVisitor, ImplCandidate, UnsatisfiedConst, + ArgKind, CandidateSimilarity, GetSafeTransmuteErrorAndReason, ImplCandidate, UnsatisfiedConst, }; pub use rustc_infer::traits::error_reporting::*; @@ -956,7 +951,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } /// When the `E` of the resulting `Result` in an expression `foo().bar().baz()?`, - /// identify thoe method chain sub-expressions that could or could not have been annotated + /// identify those method chain sub-expressions that could or could not have been annotated /// with `?`. fn try_conversion_context( &self, @@ -2172,536 +2167,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { Obligation::new(self.tcx, ObligationCause::dummy(), param_env, trait_pred) } - #[instrument(skip(self), level = "debug")] - fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>) -> ErrorGuaranteed { - // Unable to successfully determine, probably means - // insufficient type information, but could mean - // ambiguous impls. The latter *ought* to be a - // coherence violation, so we don't report it here. - - let predicate = self.resolve_vars_if_possible(obligation.predicate); - let span = obligation.cause.span; - - debug!(?predicate, obligation.cause.code = ?obligation.cause.code()); - - // Ambiguity errors are often caused as fallout from earlier errors. - // We ignore them if this `infcx` is tainted in some cases below. - - let bound_predicate = predicate.kind(); - let mut err = match bound_predicate.skip_binder() { - ty::PredicateKind::Clause(ty::ClauseKind::Trait(data)) => { - let trait_ref = bound_predicate.rebind(data.trait_ref); - debug!(?trait_ref); - - if let Err(e) = predicate.error_reported() { - return e; - } - - if let Err(guar) = self.tcx.ensure().coherent_trait(trait_ref.def_id()) { - // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case - // other `Foo` impls are incoherent. - return guar; - } - - // This is kind of a hack: it frequently happens that some earlier - // error prevents types from being fully inferred, and then we get - // a bunch of uninteresting errors saying something like " doesn't implement Sized". It may even be true that we - // could just skip over all checks where the self-ty is an - // inference variable, but I was afraid that there might be an - // inference variable created, registered as an obligation, and - // then never forced by writeback, and hence by skipping here we'd - // be ignoring the fact that we don't KNOW the type works - // out. Though even that would probably be harmless, given that - // we're only talking about builtin traits, which are known to be - // inhabited. We used to check for `self.tcx.sess.has_errors()` to - // avoid inundating the user with unnecessary errors, but we now - // check upstream for type errors and don't add the obligations to - // begin with in those cases. - if self.tcx.is_lang_item(trait_ref.def_id(), LangItem::Sized) { - match self.tainted_by_errors() { - None => { - let err = self.emit_inference_failure_err( - obligation.cause.body_id, - span, - trait_ref.self_ty().skip_binder().into(), - ErrorCode::E0282, - false, - ); - return err.stash(span, StashKey::MaybeForgetReturn).unwrap(); - } - Some(e) => return e, - } - } - - // Typically, this ambiguity should only happen if - // there are unresolved type inference variables - // (otherwise it would suggest a coherence - // failure). But given #21974 that is not necessarily - // the case -- we can have multiple where clauses that - // are only distinguished by a region, which results - // in an ambiguity even when all types are fully - // known, since we don't dispatch based on region - // relationships. - - // Pick the first generic parameter that still contains inference variables as the one - // we're going to emit an error for. If there are none (see above), fall back to - // a more general error. - let arg = data.trait_ref.args.iter().find(|s| s.has_non_region_infer()); - - let mut err = if let Some(arg) = arg { - self.emit_inference_failure_err( - obligation.cause.body_id, - span, - arg, - ErrorCode::E0283, - true, - ) - } else { - struct_span_code_err!( - self.dcx(), - span, - E0283, - "type annotations needed: cannot satisfy `{}`", - predicate, - ) - }; - - let mut ambiguities = ambiguity::compute_applicable_impls_for_diagnostics( - self.infcx, - &obligation.with(self.tcx, trait_ref), - ); - let has_non_region_infer = - trait_ref.skip_binder().args.types().any(|t| !t.is_ty_or_numeric_infer()); - // It doesn't make sense to talk about applicable impls if there are more than a - // handful of them. If there are a lot of them, but only a few of them have no type - // params, we only show those, as they are more likely to be useful/intended. - if ambiguities.len() > 5 { - let infcx = self.infcx; - if !ambiguities.iter().all(|option| match option { - DefId(did) => infcx.tcx.generics_of(*did).count() == 0, - ParamEnv(_) => true, - }) { - // If not all are blanket impls, we filter blanked impls out. - ambiguities.retain(|option| match option { - DefId(did) => infcx.tcx.generics_of(*did).count() == 0, - ParamEnv(_) => true, - }); - } - } - if ambiguities.len() > 1 && ambiguities.len() < 10 && has_non_region_infer { - if let Some(e) = self.tainted_by_errors() - && arg.is_none() - { - // If `arg.is_none()`, then this is probably two param-env - // candidates or impl candidates that are equal modulo lifetimes. - // Therefore, if we've already emitted an error, just skip this - // one, since it's not particularly actionable. - err.cancel(); - return e; - } - self.annotate_source_of_ambiguity(&mut err, &ambiguities, predicate); - } else { - if let Some(e) = self.tainted_by_errors() { - err.cancel(); - return e; - } - err.note(format!("cannot satisfy `{predicate}`")); - let impl_candidates = - self.find_similar_impl_candidates(predicate.as_trait_clause().unwrap()); - if impl_candidates.len() < 40 { - self.report_similar_impl_candidates( - impl_candidates.as_slice(), - trait_ref, - obligation.cause.body_id, - &mut err, - false, - obligation.param_env, - ); - } - } - - if let ObligationCauseCode::WhereClause(def_id, _) - | ObligationCauseCode::WhereClauseInExpr(def_id, ..) = *obligation.cause.code() - { - self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id()); - } - - if let Some(ty::GenericArgKind::Type(_)) = arg.map(|arg| arg.unpack()) - && let Some(body) = self.tcx.hir().maybe_body_owned_by(obligation.cause.body_id) - { - let mut expr_finder = FindExprBySpan::new(span, self.tcx); - expr_finder.visit_expr(&body.value); - - if let Some(hir::Expr { - kind: - hir::ExprKind::Call( - hir::Expr { - kind: hir::ExprKind::Path(hir::QPath::Resolved(None, path)), - .. - }, - _, - ) - | hir::ExprKind::Path(hir::QPath::Resolved(None, path)), - .. - }) = expr_finder.result - && let [ - .., - trait_path_segment @ hir::PathSegment { - res: Res::Def(DefKind::Trait, trait_id), - .. - }, - hir::PathSegment { - ident: assoc_item_name, - res: Res::Def(_, item_id), - .. - }, - ] = path.segments - && data.trait_ref.def_id == *trait_id - && self.tcx.trait_of_item(*item_id) == Some(*trait_id) - && let None = self.tainted_by_errors() - { - let (verb, noun) = match self.tcx.associated_item(item_id).kind { - ty::AssocKind::Const => ("refer to the", "constant"), - ty::AssocKind::Fn => ("call", "function"), - // This is already covered by E0223, but this following single match - // arm doesn't hurt here. - ty::AssocKind::Type => ("refer to the", "type"), - }; - - // Replace the more general E0283 with a more specific error - err.cancel(); - err = self.dcx().struct_span_err( - span, - format!( - "cannot {verb} associated {noun} on trait without specifying the \ - corresponding `impl` type", - ), - ); - err.code(E0790); - - if let Some(local_def_id) = data.trait_ref.def_id.as_local() - && let hir::Node::Item(hir::Item { - ident: trait_name, - kind: hir::ItemKind::Trait(_, _, _, _, trait_item_refs), - .. - }) = self.tcx.hir_node_by_def_id(local_def_id) - && let Some(method_ref) = trait_item_refs - .iter() - .find(|item_ref| item_ref.ident == *assoc_item_name) - { - err.span_label( - method_ref.span, - format!("`{trait_name}::{assoc_item_name}` defined here"), - ); - } - - err.span_label(span, format!("cannot {verb} associated {noun} of trait")); - - let trait_impls = self.tcx.trait_impls_of(data.trait_ref.def_id); - - if let Some(impl_def_id) = - trait_impls.non_blanket_impls().values().flatten().next() - { - let non_blanket_impl_count = - trait_impls.non_blanket_impls().values().flatten().count(); - // If there is only one implementation of the trait, suggest using it. - // Otherwise, use a placeholder comment for the implementation. - let (message, self_type) = if non_blanket_impl_count == 1 { - ( - "use the fully-qualified path to the only available \ - implementation", - format!( - "{}", - self.tcx.type_of(impl_def_id).instantiate_identity() - ), - ) - } else { - ( - "use a fully-qualified path to a specific available \ - implementation", - "/* self type */".to_string(), - ) - }; - let mut suggestions = - vec![(path.span.shrink_to_lo(), format!("<{self_type} as "))]; - if let Some(generic_arg) = trait_path_segment.args { - let between_span = - trait_path_segment.ident.span.between(generic_arg.span_ext); - // get rid of :: between Trait and - // must be '::' between them, otherwise the parser won't accept the code - suggestions.push((between_span, "".to_string())); - suggestions - .push((generic_arg.span_ext.shrink_to_hi(), ">".to_string())); - } else { - suggestions.push(( - trait_path_segment.ident.span.shrink_to_hi(), - ">".to_string(), - )); - } - err.multipart_suggestion( - message, - suggestions, - Applicability::MaybeIncorrect, - ); - } - } - }; - - err - } - - ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(arg)) => { - // Same hacky approach as above to avoid deluging user - // with error messages. - - if let Err(e) = arg.error_reported() { - return e; - } - if let Some(e) = self.tainted_by_errors() { - return e; - } - - self.emit_inference_failure_err( - obligation.cause.body_id, - span, - arg, - ErrorCode::E0282, - false, - ) - } - - ty::PredicateKind::Subtype(data) => { - if let Err(e) = data.error_reported() { - return e; - } - if let Some(e) = self.tainted_by_errors() { - return e; - } - let SubtypePredicate { a_is_expected: _, a, b } = data; - // both must be type variables, or the other would've been instantiated - assert!(a.is_ty_var() && b.is_ty_var()); - self.emit_inference_failure_err( - obligation.cause.body_id, - span, - a.into(), - ErrorCode::E0282, - true, - ) - } - ty::PredicateKind::Clause(ty::ClauseKind::Projection(data)) => { - if let Err(e) = predicate.error_reported() { - return e; - } - if let Some(e) = self.tainted_by_errors() { - return e; - } - - if let Err(guar) = - self.tcx.ensure().coherent_trait(self.tcx.parent(data.projection_term.def_id)) - { - // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case - // other `Foo` impls are incoherent. - return guar; - } - let arg = data - .projection_term - .args - .iter() - .chain(Some(data.term.into_arg())) - .find(|g| g.has_non_region_infer()); - if let Some(arg) = arg { - self.emit_inference_failure_err( - obligation.cause.body_id, - span, - arg, - ErrorCode::E0284, - true, - ) - .with_note(format!("cannot satisfy `{predicate}`")) - } else { - // If we can't find a generic parameter, just print a generic error - struct_span_code_err!( - self.dcx(), - span, - E0284, - "type annotations needed: cannot satisfy `{}`", - predicate, - ) - .with_span_label(span, format!("cannot satisfy `{predicate}`")) - } - } - - ty::PredicateKind::Clause(ty::ClauseKind::ConstEvaluatable(data)) => { - if let Err(e) = predicate.error_reported() { - return e; - } - if let Some(e) = self.tainted_by_errors() { - return e; - } - let arg = data.walk().find(|g| g.is_non_region_infer()); - if let Some(arg) = arg { - let err = self.emit_inference_failure_err( - obligation.cause.body_id, - span, - arg, - ErrorCode::E0284, - true, - ); - err - } else { - // If we can't find a generic parameter, just print a generic error - struct_span_code_err!( - self.dcx(), - span, - E0284, - "type annotations needed: cannot satisfy `{}`", - predicate, - ) - .with_span_label(span, format!("cannot satisfy `{predicate}`")) - } - } - - ty::PredicateKind::Clause(ty::ClauseKind::ConstArgHasType(ct, ..)) => self - .emit_inference_failure_err( - obligation.cause.body_id, - span, - ct.into(), - ErrorCode::E0284, - true, - ), - ty::PredicateKind::NormalizesTo(ty::NormalizesTo { alias, term }) - if term.is_infer() => - { - if let Some(e) = self.tainted_by_errors() { - return e; - } - struct_span_code_err!( - self.dcx(), - span, - E0284, - "type annotations needed: cannot normalize `{alias}`", - ) - .with_span_label(span, format!("cannot normalize `{alias}`")) - } - - _ => { - if let Some(e) = self.tainted_by_errors() { - return e; - } - struct_span_code_err!( - self.dcx(), - span, - E0284, - "type annotations needed: cannot satisfy `{}`", - predicate, - ) - .with_span_label(span, format!("cannot satisfy `{predicate}`")) - } - }; - self.note_obligation_cause(&mut err, obligation); - err.emit() - } - - fn annotate_source_of_ambiguity( - &self, - err: &mut Diag<'_>, - ambiguities: &[ambiguity::CandidateSource], - predicate: ty::Predicate<'tcx>, - ) { - let mut spans = vec![]; - let mut crates = vec![]; - let mut post = vec![]; - let mut has_param_env = false; - for ambiguity in ambiguities { - match ambiguity { - ambiguity::CandidateSource::DefId(impl_def_id) => { - match self.tcx.span_of_impl(*impl_def_id) { - Ok(span) => spans.push(span), - Err(name) => { - crates.push(name); - if let Some(header) = to_pretty_impl_header(self.tcx, *impl_def_id) { - post.push(header); - } - } - } - } - ambiguity::CandidateSource::ParamEnv(span) => { - has_param_env = true; - spans.push(*span); - } - } - } - let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{n}`")).collect(); - crate_names.sort(); - crate_names.dedup(); - post.sort(); - post.dedup(); - - if self.tainted_by_errors().is_some() - && (crate_names.len() == 1 - && spans.len() == 0 - && ["`core`", "`alloc`", "`std`"].contains(&crate_names[0].as_str()) - || predicate.visit_with(&mut HasNumericInferVisitor).is_break()) - { - // Avoid complaining about other inference issues for expressions like - // `42 >> 1`, where the types are still `{integer}`, but we want to - // Do we need `trait_ref.skip_binder().self_ty().is_numeric() &&` too? - // NOTE(eddyb) this was `.cancel()`, but `err` - // is borrowed, so we can't fully defuse it. - err.downgrade_to_delayed_bug(); - return; - } - - let msg = format!( - "multiple `impl`s{} satisfying `{}` found", - if has_param_env { " or `where` clauses" } else { "" }, - predicate - ); - let post = if post.len() > 1 || (post.len() == 1 && post[0].contains('\n')) { - format!(":\n{}", post.iter().map(|p| format!("- {p}")).collect::>().join("\n"),) - } else if post.len() == 1 { - format!(": `{}`", post[0]) - } else { - String::new() - }; - - match (spans.len(), crates.len(), crate_names.len()) { - (0, 0, 0) => { - err.note(format!("cannot satisfy `{predicate}`")); - } - (0, _, 1) => { - err.note(format!("{} in the `{}` crate{}", msg, crates[0], post,)); - } - (0, _, _) => { - err.note(format!( - "{} in the following crates: {}{}", - msg, - crate_names.join(", "), - post, - )); - } - (_, 0, 0) => { - let span: MultiSpan = spans.into(); - err.span_note(span, msg); - } - (_, 1, 1) => { - let span: MultiSpan = spans.into(); - err.span_note(span, msg); - err.note(format!("and another `impl` found in the `{}` crate{}", crates[0], post,)); - } - _ => { - let span: MultiSpan = spans.into(); - err.span_note(span, msg); - err.note(format!( - "and more `impl`s found in the following crates: {}{}", - crate_names.join(", "), - post, - )); - } - } - } - /// Returns `true` if the trait predicate may apply for *some* assignment /// to the type parameters. fn predicate_can_apply( @@ -2769,136 +2234,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } } - #[instrument(level = "debug", skip_all)] - fn suggest_unsized_bound_if_applicable( - &self, - err: &mut Diag<'_>, - obligation: &PredicateObligation<'tcx>, - ) { - let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) = - obligation.predicate.kind().skip_binder() - else { - return; - }; - let (ObligationCauseCode::WhereClause(item_def_id, span) - | ObligationCauseCode::WhereClauseInExpr(item_def_id, span, ..)) = - *obligation.cause.code().peel_derives() - else { - return; - }; - if span.is_dummy() { - return; - } - debug!(?pred, ?item_def_id, ?span); - - let (Some(node), true) = ( - self.tcx.hir().get_if_local(item_def_id), - self.tcx.is_lang_item(pred.def_id(), LangItem::Sized), - ) else { - return; - }; - self.maybe_suggest_unsized_generics(err, span, node); - } - - #[instrument(level = "debug", skip_all)] - fn maybe_suggest_unsized_generics(&self, err: &mut Diag<'_>, span: Span, node: Node<'tcx>) { - let Some(generics) = node.generics() else { - return; - }; - let sized_trait = self.tcx.lang_items().sized_trait(); - debug!(?generics.params); - debug!(?generics.predicates); - let Some(param) = generics.params.iter().find(|param| param.span == span) else { - return; - }; - // Check that none of the explicit trait bounds is `Sized`. Assume that an explicit - // `Sized` bound is there intentionally and we don't need to suggest relaxing it. - let explicitly_sized = generics - .bounds_for_param(param.def_id) - .flat_map(|bp| bp.bounds) - .any(|bound| bound.trait_ref().and_then(|tr| tr.trait_def_id()) == sized_trait); - if explicitly_sized { - return; - } - debug!(?param); - match node { - hir::Node::Item( - item @ hir::Item { - // Only suggest indirection for uses of type parameters in ADTs. - kind: - hir::ItemKind::Enum(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Union(..), - .. - }, - ) => { - if self.maybe_indirection_for_unsized(err, item, param) { - return; - } - } - _ => {} - }; - - // Didn't add an indirection suggestion, so add a general suggestion to relax `Sized`. - let (span, separator, open_paren_sp) = - if let Some((s, open_paren_sp)) = generics.bounds_span_for_suggestions(param.def_id) { - (s, " +", open_paren_sp) - } else { - (param.name.ident().span.shrink_to_hi(), ":", None) - }; - - let mut suggs = vec![]; - let suggestion = format!("{separator} ?Sized"); - - if let Some(open_paren_sp) = open_paren_sp { - suggs.push((open_paren_sp, "(".to_string())); - suggs.push((span, format!("){suggestion}"))); - } else { - suggs.push((span, suggestion)); - } - - err.multipart_suggestion_verbose( - "consider relaxing the implicit `Sized` restriction", - suggs, - Applicability::MachineApplicable, - ); - } - - fn maybe_indirection_for_unsized( - &self, - err: &mut Diag<'_>, - item: &Item<'tcx>, - param: &GenericParam<'tcx>, - ) -> bool { - // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a - // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S(T);` - // is not. Look for invalid "bare" parameter uses, and suggest using indirection. - let mut visitor = - FindTypeParam { param: param.name.ident().name, invalid_spans: vec![], nested: false }; - visitor.visit_item(item); - if visitor.invalid_spans.is_empty() { - return false; - } - let mut multispan: MultiSpan = param.span.into(); - multispan.push_span_label( - param.span, - format!("this could be changed to `{}: ?Sized`...", param.name.ident()), - ); - for sp in visitor.invalid_spans { - multispan.push_span_label( - sp, - format!("...if indirection were used here: `Box<{}>`", param.name.ident()), - ); - } - err.span_help( - multispan, - format!( - "you could relax the implicit `Sized` bound on `{T}` if it were \ - used through indirection like `&{T}` or `Box<{T}>`", - T = param.name.ident(), - ), - ); - true - } - fn is_recursive_obligation( &self, obligated_types: &mut Vec>, From bbbff806038eeb178a7984820f498a03baff02f1 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 8 Jul 2024 16:58:18 -0400 Subject: [PATCH 3/3] Split out fulfillment error reporting a bit more --- compiler/rustc_hir_typeck/src/coercion.rs | 2 +- ..._err_ctxt_ext.rs => fulfillment_errors.rs} | 204 +---------------- .../error_reporting/traits/infer_ctxt_ext.rs | 4 +- .../src/error_reporting/traits/mod.rs | 215 +++++++++++++++++- .../traits/on_unimplemented.rs | 2 +- .../src/error_reporting/traits/suggestions.rs | 2 +- 6 files changed, 221 insertions(+), 208 deletions(-) rename compiler/rustc_trait_selection/src/error_reporting/traits/{type_err_ctxt_ext.rs => fulfillment_errors.rs} (93%) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index d1b99aae25b1d..5e2a68e1f02db 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -59,7 +59,7 @@ use rustc_span::symbol::sym; use rustc_span::{BytePos, DesugaringKind, Span, DUMMY_SP}; use rustc_target::spec::abi::Abi; use rustc_trait_selection::error_reporting::traits::suggestions::TypeErrCtxtExt; -use rustc_trait_selection::error_reporting::traits::TypeErrCtxtExt as _; +use rustc_trait_selection::error_reporting::traits::TypeErrCtxtSelectionErrExt as _; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::{ diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs similarity index 93% rename from compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs rename to compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index dec3b26420c24..f7ec5f1ff325f 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -1,8 +1,6 @@ -use super::ambiguity::TypeErrCtxtAmbiguityExt as _; use super::on_unimplemented::{AppendConstMessage, OnUnimplementedNote, TypeErrCtxtExt as _}; use super::suggestions::{get_explanation_based_on_obligation, TypeErrCtxtExt as _}; use crate::error_reporting::traits::infer_ctxt_ext::InferCtxtExt; -use crate::error_reporting::traits::overflow::TypeErrCtxtOverflowExt; use crate::errors::{ AsyncClosureNotFn, ClosureFnMutLabel, ClosureFnOnceLabel, ClosureKindMismatch, }; @@ -12,12 +10,12 @@ use crate::infer::{self, InferCtxt}; use crate::traits::query::evaluate_obligation::InferCtxtExt as _; use crate::traits::NormalizeExt; use crate::traits::{ - elaborate, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes, Obligation, - ObligationCause, ObligationCauseCode, ObligationCtxt, Overflow, PredicateObligation, - SelectionError, SignatureMismatch, TraitNotObjectSafe, + elaborate, MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode, + ObligationCtxt, Overflow, PredicateObligation, SelectionError, SignatureMismatch, + TraitNotObjectSafe, }; use core::ops::ControlFlow; -use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::unord::UnordSet; use rustc_errors::codes::*; use rustc_errors::{pluralize, struct_span_code_err, Applicability, StringPart}; @@ -44,9 +42,8 @@ use rustc_middle::ty::{ }; use rustc_middle::{bug, span_bug}; use rustc_span::symbol::sym; -use rustc_span::{BytePos, ExpnKind, Span, Symbol, DUMMY_SP}; +use rustc_span::{BytePos, Span, Symbol, DUMMY_SP}; use std::borrow::Cow; -use std::iter; use super::{ ArgKind, CandidateSimilarity, GetSafeTransmuteErrorAndReason, ImplCandidate, UnsatisfiedConst, @@ -54,128 +51,8 @@ use super::{ pub use rustc_infer::traits::error_reporting::*; -#[extension(pub trait TypeErrCtxtExt<'a, 'tcx>)] +#[extension(pub trait TypeErrCtxtSelectionErrExt<'a, 'tcx>)] impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { - fn report_fulfillment_errors( - &self, - mut errors: Vec>, - ) -> ErrorGuaranteed { - self.sub_relations - .borrow_mut() - .add_constraints(self, errors.iter().map(|e| e.obligation.predicate)); - - #[derive(Debug)] - struct ErrorDescriptor<'tcx> { - predicate: ty::Predicate<'tcx>, - index: Option, // None if this is an old error - } - - let mut error_map: FxIndexMap<_, Vec<_>> = self - .reported_trait_errors - .borrow() - .iter() - .map(|(&span, predicates)| { - ( - span, - predicates - .0 - .iter() - .map(|&predicate| ErrorDescriptor { predicate, index: None }) - .collect(), - ) - }) - .collect(); - - // Ensure `T: Sized` and `T: WF` obligations come last. This lets us display diagnostics - // with more relevant type information and hide redundant E0282 errors. - errors.sort_by_key(|e| match e.obligation.predicate.kind().skip_binder() { - ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) - if self.tcx.is_lang_item(pred.def_id(), LangItem::Sized) => - { - 1 - } - ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(_)) => 3, - ty::PredicateKind::Coerce(_) => 2, - _ => 0, - }); - - for (index, error) in errors.iter().enumerate() { - // We want to ignore desugarings here: spans are equivalent even - // if one is the result of a desugaring and the other is not. - let mut span = error.obligation.cause.span; - let expn_data = span.ctxt().outer_expn_data(); - if let ExpnKind::Desugaring(_) = expn_data.kind { - span = expn_data.call_site; - } - - error_map.entry(span).or_default().push(ErrorDescriptor { - predicate: error.obligation.predicate, - index: Some(index), - }); - } - - // We do this in 2 passes because we want to display errors in order, though - // maybe it *is* better to sort errors by span or something. - let mut is_suppressed = vec![false; errors.len()]; - for (_, error_set) in error_map.iter() { - // We want to suppress "duplicate" errors with the same span. - for error in error_set { - if let Some(index) = error.index { - // Suppress errors that are either: - // 1) strictly implied by another error. - // 2) implied by an error with a smaller index. - for error2 in error_set { - if error2.index.is_some_and(|index2| is_suppressed[index2]) { - // Avoid errors being suppressed by already-suppressed - // errors, to prevent all errors from being suppressed - // at once. - continue; - } - - if self.error_implies(error2.predicate, error.predicate) - && !(error2.index >= error.index - && self.error_implies(error.predicate, error2.predicate)) - { - info!("skipping {:?} (implied by {:?})", error, error2); - is_suppressed[index] = true; - break; - } - } - } - } - } - - let mut reported = None; - - for from_expansion in [false, true] { - for (error, suppressed) in iter::zip(&errors, &is_suppressed) { - if !suppressed && error.obligation.cause.span.from_expansion() == from_expansion { - let guar = self.report_fulfillment_error(error); - self.infcx.set_tainted_by_errors(guar); - reported = Some(guar); - // We want to ignore desugarings here: spans are equivalent even - // if one is the result of a desugaring and the other is not. - let mut span = error.obligation.cause.span; - let expn_data = span.ctxt().outer_expn_data(); - if let ExpnKind::Desugaring(_) = expn_data.kind { - span = expn_data.call_site; - } - self.reported_trait_errors - .borrow_mut() - .entry(span) - .or_insert_with(|| (vec![], guar)) - .0 - .push(error.obligation.predicate); - } - } - } - - // It could be that we don't report an error because we have seen an `ErrorReported` from - // another source. We should probably be able to fix most of these, but some are delayed - // bugs that get a proper error after this function. - reported.unwrap_or_else(|| self.dcx().delayed_bug("failed to report fulfillment errors")) - } - /// The `root_obligation` parameter should be the `root_obligation` field /// from a `FulfillmentError`. If no `FulfillmentError` is available, /// then it should be the same as `obligation`. @@ -803,7 +680,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { self.point_at_returns_when_relevant(&mut err, &obligation); err.emit() } +} +#[extension(pub(super) trait TypeErrCtxtExt<'a, 'tcx>)] +impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { fn apply_do_not_recommend(&self, obligation: &mut PredicateObligation<'tcx>) -> bool { let mut base_cause = obligation.cause.code().clone(); let mut applied_do_not_recommend = false; @@ -1324,72 +1204,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } } - #[instrument(skip(self), level = "debug")] - fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) -> ErrorGuaranteed { - let mut error = FulfillmentError { - obligation: error.obligation.clone(), - code: error.code.clone(), - root_obligation: error.root_obligation.clone(), - }; - if matches!( - error.code, - FulfillmentErrorCode::Select(crate::traits::SelectionError::Unimplemented) - | FulfillmentErrorCode::Project(_) - ) && self.apply_do_not_recommend(&mut error.obligation) - { - error.code = FulfillmentErrorCode::Select(SelectionError::Unimplemented); - } - - match error.code { - FulfillmentErrorCode::Select(ref selection_error) => self.report_selection_error( - error.obligation.clone(), - &error.root_obligation, - selection_error, - ), - FulfillmentErrorCode::Project(ref e) => { - self.report_projection_error(&error.obligation, e) - } - FulfillmentErrorCode::Ambiguity { overflow: None } => { - self.maybe_report_ambiguity(&error.obligation) - } - FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) } => { - self.report_overflow_no_abort(error.obligation.clone(), suggest_increasing_limit) - } - FulfillmentErrorCode::Subtype(ref expected_found, ref err) => self - .report_mismatched_types( - &error.obligation.cause, - expected_found.expected, - expected_found.found, - *err, - ) - .emit(), - FulfillmentErrorCode::ConstEquate(ref expected_found, ref err) => { - let mut diag = self.report_mismatched_consts( - &error.obligation.cause, - expected_found.expected, - expected_found.found, - *err, - ); - let code = error.obligation.cause.code().peel_derives().peel_match_impls(); - if let ObligationCauseCode::WhereClause(..) - | ObligationCauseCode::WhereClauseInExpr(..) = code - { - self.note_obligation_cause_code( - error.obligation.cause.body_id, - &mut diag, - error.obligation.predicate, - error.obligation.param_env, - code, - &mut vec![], - &mut Default::default(), - ); - } - diag.emit() - } - FulfillmentErrorCode::Cycle(ref cycle) => self.report_overflow_obligation_cycle(cycle), - } - } - #[instrument(level = "debug", skip_all)] fn report_projection_error( &self, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/infer_ctxt_ext.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/infer_ctxt_ext.rs index 34da8e576ce86..e8d7e80ac562c 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/infer_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/infer_ctxt_ext.rs @@ -1,3 +1,5 @@ +// FIXME(error_reporting): This should be made into private methods on `TypeErrCtxt`. + use crate::infer::InferCtxt; use crate::traits::{Obligation, ObligationCause, ObligationCtxt}; use rustc_errors::{codes::*, pluralize, struct_span_code_err, Applicability, Diag}; @@ -9,8 +11,6 @@ use rustc_span::{Span, DUMMY_SP}; use super::ArgKind; -pub use rustc_infer::traits::error_reporting::*; - #[extension(pub trait InferCtxtExt<'tcx>)] impl<'tcx> InferCtxt<'tcx> { /// Given some node representing a fn-like thing in the HIR map, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs index e776248e68476..2131e236401ef 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs @@ -1,24 +1,34 @@ -// ignore-tidy-filelength :( - pub mod ambiguity; +mod fulfillment_errors; mod infer_ctxt_ext; pub mod on_unimplemented; mod overflow; pub mod suggestions; -mod type_err_ctxt_ext; -use rustc_data_structures::fx::FxIndexSet; -use rustc_hir as hir; +use std::iter; + +use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_infer::traits::{Obligation, ObligationCause, ObligationCauseCode, PredicateObligation}; +use rustc_hir::{self as hir, LangItem}; +use rustc_infer::infer::error_reporting::TypeErrCtxt; +use rustc_infer::traits::{ + Obligation, ObligationCause, ObligationCauseCode, PredicateObligation, SelectionError, +}; +use rustc_macros::extension; use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_span::Span; +use rustc_span::{ErrorGuaranteed, ExpnKind, Span}; + +use ambiguity::TypeErrCtxtAmbiguityExt as _; +use fulfillment_errors::TypeErrCtxtExt as _; +use suggestions::TypeErrCtxtExt as _; +use crate::traits::{FulfillmentError, FulfillmentErrorCode}; + +pub use self::fulfillment_errors::*; pub use self::infer_ctxt_ext::*; pub use self::overflow::*; -pub use self::type_err_ctxt_ext::*; // When outputting impl candidates, prefer showing those that are more similar. // @@ -127,6 +137,195 @@ pub enum DefIdOrName { Name(&'static str), } +#[extension(pub trait TypeErrCtxtExt<'a, 'tcx>)] +impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { + fn report_fulfillment_errors( + &self, + mut errors: Vec>, + ) -> ErrorGuaranteed { + self.sub_relations + .borrow_mut() + .add_constraints(self, errors.iter().map(|e| e.obligation.predicate)); + + #[derive(Debug)] + struct ErrorDescriptor<'tcx> { + predicate: ty::Predicate<'tcx>, + index: Option, // None if this is an old error + } + + let mut error_map: FxIndexMap<_, Vec<_>> = self + .reported_trait_errors + .borrow() + .iter() + .map(|(&span, predicates)| { + ( + span, + predicates + .0 + .iter() + .map(|&predicate| ErrorDescriptor { predicate, index: None }) + .collect(), + ) + }) + .collect(); + + // Ensure `T: Sized` and `T: WF` obligations come last. This lets us display diagnostics + // with more relevant type information and hide redundant E0282 errors. + errors.sort_by_key(|e| match e.obligation.predicate.kind().skip_binder() { + ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) + if self.tcx.is_lang_item(pred.def_id(), LangItem::Sized) => + { + 1 + } + ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(_)) => 3, + ty::PredicateKind::Coerce(_) => 2, + _ => 0, + }); + + for (index, error) in errors.iter().enumerate() { + // We want to ignore desugarings here: spans are equivalent even + // if one is the result of a desugaring and the other is not. + let mut span = error.obligation.cause.span; + let expn_data = span.ctxt().outer_expn_data(); + if let ExpnKind::Desugaring(_) = expn_data.kind { + span = expn_data.call_site; + } + + error_map.entry(span).or_default().push(ErrorDescriptor { + predicate: error.obligation.predicate, + index: Some(index), + }); + } + + // We do this in 2 passes because we want to display errors in order, though + // maybe it *is* better to sort errors by span or something. + let mut is_suppressed = vec![false; errors.len()]; + for (_, error_set) in error_map.iter() { + // We want to suppress "duplicate" errors with the same span. + for error in error_set { + if let Some(index) = error.index { + // Suppress errors that are either: + // 1) strictly implied by another error. + // 2) implied by an error with a smaller index. + for error2 in error_set { + if error2.index.is_some_and(|index2| is_suppressed[index2]) { + // Avoid errors being suppressed by already-suppressed + // errors, to prevent all errors from being suppressed + // at once. + continue; + } + + if self.error_implies(error2.predicate, error.predicate) + && !(error2.index >= error.index + && self.error_implies(error.predicate, error2.predicate)) + { + info!("skipping {:?} (implied by {:?})", error, error2); + is_suppressed[index] = true; + break; + } + } + } + } + } + + let mut reported = None; + + for from_expansion in [false, true] { + for (error, suppressed) in iter::zip(&errors, &is_suppressed) { + if !suppressed && error.obligation.cause.span.from_expansion() == from_expansion { + let guar = self.report_fulfillment_error(error); + self.infcx.set_tainted_by_errors(guar); + reported = Some(guar); + // We want to ignore desugarings here: spans are equivalent even + // if one is the result of a desugaring and the other is not. + let mut span = error.obligation.cause.span; + let expn_data = span.ctxt().outer_expn_data(); + if let ExpnKind::Desugaring(_) = expn_data.kind { + span = expn_data.call_site; + } + self.reported_trait_errors + .borrow_mut() + .entry(span) + .or_insert_with(|| (vec![], guar)) + .0 + .push(error.obligation.predicate); + } + } + } + + // It could be that we don't report an error because we have seen an `ErrorReported` from + // another source. We should probably be able to fix most of these, but some are delayed + // bugs that get a proper error after this function. + reported.unwrap_or_else(|| self.dcx().delayed_bug("failed to report fulfillment errors")) + } + + #[instrument(skip(self), level = "debug")] + fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) -> ErrorGuaranteed { + let mut error = FulfillmentError { + obligation: error.obligation.clone(), + code: error.code.clone(), + root_obligation: error.root_obligation.clone(), + }; + if matches!( + error.code, + FulfillmentErrorCode::Select(crate::traits::SelectionError::Unimplemented) + | FulfillmentErrorCode::Project(_) + ) && self.apply_do_not_recommend(&mut error.obligation) + { + error.code = FulfillmentErrorCode::Select(SelectionError::Unimplemented); + } + + match error.code { + FulfillmentErrorCode::Select(ref selection_error) => self.report_selection_error( + error.obligation.clone(), + &error.root_obligation, + selection_error, + ), + FulfillmentErrorCode::Project(ref e) => { + self.report_projection_error(&error.obligation, e) + } + FulfillmentErrorCode::Ambiguity { overflow: None } => { + self.maybe_report_ambiguity(&error.obligation) + } + FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) } => { + self.report_overflow_no_abort(error.obligation.clone(), suggest_increasing_limit) + } + FulfillmentErrorCode::Subtype(ref expected_found, ref err) => self + .report_mismatched_types( + &error.obligation.cause, + expected_found.expected, + expected_found.found, + *err, + ) + .emit(), + FulfillmentErrorCode::ConstEquate(ref expected_found, ref err) => { + let mut diag = self.report_mismatched_consts( + &error.obligation.cause, + expected_found.expected, + expected_found.found, + *err, + ); + let code = error.obligation.cause.code().peel_derives().peel_match_impls(); + if let ObligationCauseCode::WhereClause(..) + | ObligationCauseCode::WhereClauseInExpr(..) = code + { + self.note_obligation_cause_code( + error.obligation.cause.body_id, + &mut diag, + error.obligation.predicate, + error.obligation.param_env, + code, + &mut vec![], + &mut Default::default(), + ); + } + diag.emit() + } + FulfillmentErrorCode::Cycle(ref cycle) => self.report_overflow_obligation_cycle(cycle), + } + } +} + /// Recovers the "impl X for Y" signature from `impl_def_id` and returns it as a /// string. pub(crate) fn to_pretty_impl_header(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Option { diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs index 83c6798ba2ed1..e90fe8fb94dd2 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/on_unimplemented.rs @@ -1,5 +1,5 @@ use super::{ObligationCauseCode, PredicateObligation}; -use crate::error_reporting::traits::type_err_ctxt_ext::InferCtxtPrivExt; +use crate::error_reporting::traits::fulfillment_errors::InferCtxtPrivExt; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, }; diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 11a2b1d323379..2cf808f962f08 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -44,7 +44,7 @@ use std::assert_matches::debug_assert_matches; use std::borrow::Cow; use std::iter; -use crate::error_reporting::traits::type_err_ctxt_ext::InferCtxtPrivExt; +use crate::error_reporting::traits::fulfillment_errors::InferCtxtPrivExt; use crate::infer::InferCtxtExt as _; use crate::traits::query::evaluate_obligation::InferCtxtExt as _; use rustc_middle::ty::print::{