From a7653337380071ebc5b83a8133121b87dc892442 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 4 Apr 2021 16:55:39 -0400 Subject: [PATCH] Add initial implementation of HIR-based WF checking for diagnostics During well-formed checking, we walk through all types 'nested' in generic arguments. For example, WF-checking `Option>` will cause us to check `MyStruct` and `u8`. However, this is done on a `rustc_middle::ty::Ty`, which has no span information. As a result, any errors that occur will have a very general span (e.g. the definintion of an associated item). This becomes a problem when macros are involved. In general, an associated type like `type MyType = Option>;` may have completely different spans for each nested type in the HIR. Using the span of the entire associated item might end up pointing to a macro invocation, even though a user-provided span is available in one of the nested types. This PR adds a framework for HIR-based well formed checking. This check is only run during error reporting, and is used to obtain a more precise span for an existing error. This is accomplished by individually checking each 'nested' type in the HIR for the type, allowing us to find the most-specific type (and span) that produces a given error. The majority of the changes are to the error-reporting code. However, some of the general trait code is modified to pass through more information. Since this has no soundness implications, I've implemented a minimal version to begin with, which can be extended over time. In particular, this only works for HIR items with a corresponding `DefId` (e.g. it will not work for WF-checking performed within function bodies). --- compiler/rustc_infer/src/traits/mod.rs | 7 +- compiler/rustc_middle/src/query/mod.rs | 14 ++ compiler/rustc_middle/src/traits/mod.rs | 17 ++- .../src/borrow_check/type_check/mod.rs | 36 ++--- compiler/rustc_query_impl/src/keys.rs | 12 ++ compiler/rustc_trait_selection/src/infer.rs | 7 +- .../rustc_trait_selection/src/opaque_types.rs | 9 +- .../src/traits/chalk_fulfill.rs | 13 +- .../src/traits/error_reporting/mod.rs | 34 +++-- .../src/traits/error_reporting/suggestions.rs | 3 +- .../src/traits/fulfill.rs | 8 +- .../src/traits/select/mod.rs | 4 +- .../rustc_trait_selection/src/traits/wf.rs | 4 +- compiler/rustc_traits/src/type_op.rs | 3 +- compiler/rustc_typeck/src/check/coercion.rs | 8 +- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 31 +++- compiler/rustc_typeck/src/check/inherited.rs | 20 ++- compiler/rustc_typeck/src/check/wfcheck.rs | 23 +-- compiler/rustc_typeck/src/hir_wf_check.rs | 133 ++++++++++++++++++ compiler/rustc_typeck/src/lib.rs | 2 + src/test/ui/wf/wf-complex-assoc-type.rs | 12 ++ src/test/ui/wf/wf-complex-assoc-type.stderr | 12 ++ .../wf/wf-impl-associated-type-trait.stderr | 4 +- 23 files changed, 350 insertions(+), 66 deletions(-) create mode 100644 compiler/rustc_typeck/src/hir_wf_check.rs create mode 100644 src/test/ui/wf/wf-complex-assoc-type.rs create mode 100644 src/test/ui/wf/wf-complex-assoc-type.stderr diff --git a/compiler/rustc_infer/src/traits/mod.rs b/compiler/rustc_infer/src/traits/mod.rs index a33234a91fa..d5c17ede214 100644 --- a/compiler/rustc_infer/src/traits/mod.rs +++ b/compiler/rustc_infer/src/traits/mod.rs @@ -70,6 +70,10 @@ pub struct FulfillmentError<'tcx> { /// obligation error caused by a call argument. When this is the case, we also signal that in /// this field to ensure accuracy of suggestions. pub points_at_arg_span: bool, + /// Diagnostics only: the 'root' obligation which resulted in + /// the failure to process `obligation`. This is the obligation + /// that was initially passed to `register_predicate_obligation` + pub root_obligation: PredicateObligation<'tcx>, } #[derive(Clone)] @@ -122,8 +126,9 @@ impl<'tcx> FulfillmentError<'tcx> { pub fn new( obligation: PredicateObligation<'tcx>, code: FulfillmentErrorCode<'tcx>, + root_obligation: PredicateObligation<'tcx>, ) -> FulfillmentError<'tcx> { - FulfillmentError { obligation, code, points_at_arg_span: false } + FulfillmentError { obligation, code, points_at_arg_span: false, root_obligation } } } diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 80c4ff2ae5d..1651853a552 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1713,4 +1713,18 @@ rustc_queries! { query limits(key: ()) -> Limits { desc { "looking up limits" } } + + /// Performs an HIR-based well-formed check on the item with the given `HirId`. If + /// we get an `Umimplemented` error that matches the provided `Predicate`, return + /// the cause of the newly created obligation. + /// + /// This is only used by error-reporting code to get a better cause (in particular, a better + /// span) for an *existing* error. Therefore, it is best-effort, and may never handle + /// all of the cases that the normal `ty::Ty`-based wfcheck does. This is fine, + /// because the `ty::Ty`-based wfcheck is always run. + query diagnostic_hir_wf_check(key: (ty::Predicate<'tcx>, hir::HirId)) -> Option> { + eval_always + no_hash + desc { "performing HIR wf-checking for predicate {:?} at item {:?}", key.0, key.1 } + } } diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 892a29e4e22..221dac9ca03 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -13,6 +13,7 @@ use crate::mir::abstract_const::NotConstEvaluatable; use crate::ty::subst::SubstsRef; use crate::ty::{self, AdtKind, Ty, TyCtxt}; +use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -24,7 +25,6 @@ use smallvec::SmallVec; use std::borrow::Cow; use std::fmt; use std::ops::Deref; -use std::rc::Rc; pub use self::select::{EvaluationCache, EvaluationResult, OverflowError, SelectionCache}; @@ -87,7 +87,7 @@ pub enum Reveal { #[derive(Clone, PartialEq, Eq, Hash, Lift)] pub struct ObligationCause<'tcx> { /// `None` for `ObligationCause::dummy`, `Some` otherwise. - data: Option>>, + data: Option>>, } const DUMMY_OBLIGATION_CAUSE_DATA: ObligationCauseData<'static> = @@ -131,7 +131,7 @@ impl<'tcx> ObligationCause<'tcx> { body_id: hir::HirId, code: ObligationCauseCode<'tcx>, ) -> ObligationCause<'tcx> { - ObligationCause { data: Some(Rc::new(ObligationCauseData { span, body_id, code })) } + ObligationCause { data: Some(Lrc::new(ObligationCauseData { span, body_id, code })) } } pub fn misc(span: Span, body_id: hir::HirId) -> ObligationCause<'tcx> { @@ -148,7 +148,7 @@ impl<'tcx> ObligationCause<'tcx> { } pub fn make_mut(&mut self) -> &mut ObligationCauseData<'tcx> { - Rc::make_mut(self.data.get_or_insert_with(|| Rc::new(DUMMY_OBLIGATION_CAUSE_DATA))) + Lrc::make_mut(self.data.get_or_insert_with(|| Lrc::new(DUMMY_OBLIGATION_CAUSE_DATA))) } pub fn span(&self, tcx: TyCtxt<'tcx>) -> Span { @@ -326,6 +326,13 @@ pub enum ObligationCauseCode<'tcx> { /// If `X` is the concrete type of an opaque type `impl Y`, then `X` must implement `Y` OpaqueType, + + /// Well-formed checking. If a `HirId` is provided, + /// it is used to perform HIR-based wf checking if an error + /// occurs, in order to generate a more precise error message. + /// This is purely for diagnostic purposes - it is always + /// correct to use `MiscObligation` instead + WellFormed(Option), } impl ObligationCauseCode<'_> { @@ -389,7 +396,7 @@ pub struct DerivedObligationCause<'tcx> { pub parent_trait_ref: ty::PolyTraitRef<'tcx>, /// The parent trait had this cause. - pub parent_code: Rc>, + pub parent_code: Lrc>, } #[derive(Clone, Debug, TypeFoldable, Lift)] diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index bd951ef72b5..b4fe3313e8a 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -2070,24 +2070,26 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { debug!("check_rvalue: is_const_fn={:?}", is_const_fn); let def_id = body.source.def_id().expect_local(); - self.infcx.report_selection_error( - &traits::Obligation::new( - ObligationCause::new( - span, - self.tcx().hir().local_def_id_to_hir_id(def_id), - traits::ObligationCauseCode::RepeatVec(is_const_fn), - ), - self.param_env, - ty::Binder::dummy(ty::TraitRef::new( - self.tcx().require_lang_item( - LangItem::Copy, - Some(self.last_span), - ), - tcx.mk_substs_trait(ty, &[]), - )) - .without_const() - .to_predicate(self.tcx()), + let obligation = traits::Obligation::new( + ObligationCause::new( + span, + self.tcx().hir().local_def_id_to_hir_id(def_id), + traits::ObligationCauseCode::RepeatVec(is_const_fn), ), + self.param_env, + ty::Binder::dummy(ty::TraitRef::new( + self.tcx().require_lang_item( + LangItem::Copy, + Some(self.last_span), + ), + tcx.mk_substs_trait(ty, &[]), + )) + .without_const() + .to_predicate(self.tcx()), + ); + self.infcx.report_selection_error( + obligation.clone(), + &obligation, &traits::SelectionError::Unimplemented, false, false, diff --git a/compiler/rustc_query_impl/src/keys.rs b/compiler/rustc_query_impl/src/keys.rs index b3cc7de4662..1993e0a602f 100644 --- a/compiler/rustc_query_impl/src/keys.rs +++ b/compiler/rustc_query_impl/src/keys.rs @@ -1,6 +1,7 @@ //! Defines the set of legal keys that can be used in queries. use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; +use rustc_hir::HirId; use rustc_middle::infer::canonical::Canonical; use rustc_middle::mir; use rustc_middle::ty::fast_reject::SimplifiedType; @@ -395,3 +396,14 @@ impl<'tcx> Key for (DefId, Ty<'tcx>, SubstsRef<'tcx>, ty::ParamEnv<'tcx>) { DUMMY_SP } } + +impl<'tcx> Key for (ty::Predicate<'tcx>, HirId) { + #[inline(always)] + fn query_crate_is_local(&self) -> bool { + true + } + + fn default_span(&self, _tcx: TyCtxt<'_>) -> Span { + DUMMY_SP + } +} diff --git a/compiler/rustc_trait_selection/src/infer.rs b/compiler/rustc_trait_selection/src/infer.rs index 9fc907da265..ea074192d23 100644 --- a/compiler/rustc_trait_selection/src/infer.rs +++ b/compiler/rustc_trait_selection/src/infer.rs @@ -30,8 +30,7 @@ pub trait InferCtxtExt<'tcx> { fn partially_normalize_associated_types_in( &self, - span: Span, - body_id: hir::HirId, + cause: ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, value: T, ) -> InferOk<'tcx, T> @@ -79,8 +78,7 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { /// new obligations that must further be processed. fn partially_normalize_associated_types_in( &self, - span: Span, - body_id: hir::HirId, + cause: ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, value: T, ) -> InferOk<'tcx, T> @@ -89,7 +87,6 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> { { debug!("partially_normalize_associated_types_in(value={:?})", value); let mut selcx = traits::SelectionContext::new(self); - let cause = ObligationCause::misc(span, body_id); let traits::Normalized { value, obligations } = traits::normalize(&mut selcx, param_env, cause, value); debug!( diff --git a/compiler/rustc_trait_selection/src/opaque_types.rs b/compiler/rustc_trait_selection/src/opaque_types.rs index 0061ce4ed37..e11ac9b49f3 100644 --- a/compiler/rustc_trait_selection/src/opaque_types.rs +++ b/compiler/rustc_trait_selection/src/opaque_types.rs @@ -1,5 +1,5 @@ use crate::infer::InferCtxtExt as _; -use crate::traits::{self, PredicateObligation}; +use crate::traits::{self, ObligationCause, PredicateObligation}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; use rustc_data_structures::vec_map::VecMap; @@ -1051,8 +1051,11 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> { item_bounds.iter().map(|(bound, _)| bound.subst(tcx, substs)).collect(); let param_env = tcx.param_env(def_id); - let InferOk { value: bounds, obligations } = - infcx.partially_normalize_associated_types_in(span, self.body_id, param_env, bounds); + let InferOk { value: bounds, obligations } = infcx.partially_normalize_associated_types_in( + ObligationCause::misc(span, self.body_id), + param_env, + bounds, + ); self.obligations.extend(obligations); debug!("instantiate_opaque_types: bounds={:?}", bounds); diff --git a/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs b/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs index 026ab414443..7a690af0cc6 100644 --- a/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs @@ -58,6 +58,9 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> { obligation: obligation.clone(), code: FulfillmentErrorCode::CodeAmbiguity, points_at_arg_span: false, + // FIXME - does Chalk have a notation of 'root obligation'? + // This is just for diagnostics, so it's okay if this is wrong + root_obligation: obligation.clone(), }) .collect(); Err(errors) @@ -105,11 +108,14 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> { ), Err(_err) => errors.push(FulfillmentError { - obligation, + obligation: obligation.clone(), code: FulfillmentErrorCode::CodeSelectionError( SelectionError::Unimplemented, ), points_at_arg_span: false, + // FIXME - does Chalk have a notation of 'root obligation'? + // This is just for diagnostics, so it's okay if this is wrong + root_obligation: obligation, }), } } else { @@ -119,11 +125,14 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> { } Err(NoSolution) => errors.push(FulfillmentError { - obligation, + obligation: obligation.clone(), code: FulfillmentErrorCode::CodeSelectionError( SelectionError::Unimplemented, ), points_at_arg_span: false, + // FIXME - does Chalk have a notation of 'root obligation'? + // This is just for diagnostics, so it's okay if this is wrong + root_obligation: obligation, }), } } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index e276d92bf5a..5c4aef529e5 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -55,9 +55,13 @@ pub trait InferCtxtExt<'tcx> { fn report_overflow_error_cycle(&self, cycle: &[PredicateObligation<'tcx>]) -> !; + /// 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`. fn report_selection_error( &self, - obligation: &PredicateObligation<'tcx>, + obligation: PredicateObligation<'tcx>, + root_obligation: &PredicateObligation<'tcx>, error: &SelectionError<'tcx>, fallback_has_occurred: bool, points_at_arg: bool, @@ -225,16 +229,29 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { fn report_selection_error( &self, - obligation: &PredicateObligation<'tcx>, + mut obligation: PredicateObligation<'tcx>, + root_obligation: &PredicateObligation<'tcx>, error: &SelectionError<'tcx>, fallback_has_occurred: bool, points_at_arg: bool, ) { let tcx = self.tcx; - let span = obligation.cause.span; + let mut span = obligation.cause.span; let mut err = match *error { SelectionError::Unimplemented => { + // If this obligation was generated as a result of well-formed checking, see if we + // can get a better error message by performing HIR-based well formed checking. + if let ObligationCauseCode::WellFormed(Some(wf_hir_id)) = + root_obligation.cause.code.peel_derives() + { + if let Some(cause) = + self.tcx.diagnostic_hir_wf_check((obligation.predicate, *wf_hir_id)) + { + obligation.cause = cause; + span = obligation.cause.span; + } + } if let ObligationCauseCode::CompareImplMethodObligation { item_name, impl_item_def_id, @@ -279,7 +296,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .unwrap_or_default(); let OnUnimplementedNote { message, label, note, enclosing_scope } = - self.on_unimplemented_note(trait_ref, obligation); + self.on_unimplemented_note(trait_ref, &obligation); let have_alt_message = message.is_some() || label.is_some(); let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id()); let is_unsize = @@ -338,7 +355,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); } - if let Some(ret_span) = self.return_type_span(obligation) { + if let Some(ret_span) = self.return_type_span(&obligation) { err.span_label( ret_span, &format!( @@ -368,7 +385,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { points_at_arg, have_alt_message, ) { - self.note_obligation_cause(&mut err, obligation); + self.note_obligation_cause(&mut err, &obligation); err.emit(); return; } @@ -821,7 +838,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } }; - self.note_obligation_cause(&mut err, obligation); + self.note_obligation_cause(&mut err, &obligation); self.point_at_returns_when_relevant(&mut err, &obligation); err.emit(); @@ -1168,7 +1185,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { match error.code { FulfillmentErrorCode::CodeSelectionError(ref selection_error) => { self.report_selection_error( - &error.obligation, + error.obligation.clone(), + &error.root_obligation, selection_error, fallback_has_occurred, error.points_at_arg_span, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 26a5a65ed36..adeb1d58d1e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1902,7 +1902,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | ObligationCauseCode::ReturnNoExpression | ObligationCauseCode::UnifyReceiver(..) | ObligationCauseCode::OpaqueType - | ObligationCauseCode::MiscObligation => {} + | ObligationCauseCode::MiscObligation + | ObligationCauseCode::WellFormed(..) => {} ObligationCauseCode::SliceOrArrayElem => { err.note("slice and array elements must have `Sized` type"); } diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs index 120680092ba..21ed586ab56 100644 --- a/compiler/rustc_trait_selection/src/traits/fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs @@ -717,6 +717,10 @@ fn substs_infer_vars<'a, 'tcx>( fn to_fulfillment_error<'tcx>( error: Error, FulfillmentErrorCode<'tcx>>, ) -> FulfillmentError<'tcx> { - let obligation = error.backtrace.into_iter().next().unwrap().obligation; - FulfillmentError::new(obligation, error.error) + let mut iter = error.backtrace.into_iter(); + let obligation = iter.next().unwrap().obligation; + // The root obligation is the last item in the backtrace - if there's only + // one item, then it's the same as the main obligation + let root_obligation = iter.next_back().map_or_else(|| obligation.clone(), |e| e.obligation); + FulfillmentError::new(obligation, error.error, root_obligation) } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 708688fa8a6..f17965f6f6b 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -28,6 +28,7 @@ use crate::traits::error_reporting::InferCtxtExt; use crate::traits::project::ProjectionCacheKeyExt; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::stack::ensure_sufficient_stack; +use rustc_data_structures::sync::Lrc; use rustc_errors::ErrorReported; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -48,7 +49,6 @@ use std::cell::{Cell, RefCell}; use std::cmp; use std::fmt::{self, Display}; use std::iter; -use std::rc::Rc; pub use rustc_middle::traits::select::*; @@ -2168,7 +2168,7 @@ impl<'tcx> TraitObligationExt<'tcx> for TraitObligation<'tcx> { // by using -Z verbose or just a CLI argument. let derived_cause = DerivedObligationCause { parent_trait_ref: obligation.predicate.to_poly_trait_ref(), - parent_code: Rc::new(obligation.cause.code.clone()), + parent_code: Lrc::new(obligation.cause.code.clone()), }; let derived_code = variant(derived_cause); ObligationCause::new(obligation.cause.span, obligation.cause.body_id, derived_code) diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index f592cf1cd24..9ee6eeb1fd5 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -1,6 +1,7 @@ use crate::infer::InferCtxt; use crate::opaque_types::required_region_bounds; use crate::traits; +use rustc_data_structures::sync::Lrc; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; @@ -9,7 +10,6 @@ use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstnes use rustc_span::Span; use std::iter; -use std::rc::Rc; /// Returns the set of obligations needed to make `arg` well-formed. /// If `arg` contains unresolved inference variables, this may include /// further WF obligations. However, if `arg` IS an unresolved @@ -295,7 +295,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { if let Some(parent_trait_ref) = obligation.predicate.to_opt_poly_trait_ref() { let derived_cause = traits::DerivedObligationCause { parent_trait_ref: parent_trait_ref.value, - parent_code: Rc::new(obligation.cause.code.clone()), + parent_code: Lrc::new(obligation.cause.code.clone()), }; cause.make_mut().code = traits::ObligationCauseCode::DerivedObligation(derived_cause); diff --git a/compiler/rustc_traits/src/type_op.rs b/compiler/rustc_traits/src/type_op.rs index 6304f696b00..2c55ea7f5c1 100644 --- a/compiler/rustc_traits/src/type_op.rs +++ b/compiler/rustc_traits/src/type_op.rs @@ -67,8 +67,7 @@ impl AscribeUserTypeCx<'me, 'tcx> { { self.infcx .partially_normalize_associated_types_in( - DUMMY_SP, - hir::CRATE_HIR_ID, + ObligationCause::misc(DUMMY_SP, hir::CRATE_HIR_ID), self.param_env, value, ) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 9e89804b747..ba76b9c8dd5 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -640,7 +640,13 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { // Object safety violations or miscellaneous. Err(err) => { - self.report_selection_error(&obligation, &err, false, false); + self.report_selection_error( + obligation.clone(), + &obligation, + &err, + false, + false, + ); // Treat this like an obligation and follow through // with the unsizing - the lack of a coercion should // be silent, as it causes a type mismatch later. diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index 8e33f4f9e12..e045c30e0de 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -39,7 +39,7 @@ use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::opaque_types::InferCtxtExt as _; use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _; use rustc_trait_selection::traits::{ - self, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt, + self, ObligationCause, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt, }; use std::collections::hash_map::Entry; @@ -408,6 +408,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { value } + /// Convenience method which tracks extra diagnostic information for normalization + /// that occurs as a result of WF checking. The `hir_id` is the `HirId` of the hir item + /// whose type is being wf-checked - this is used to construct a more precise span if + /// an error occurs. + /// + /// It is never necessary to call this method - calling `normalize_associated_types_in` will + /// just result in a slightly worse diagnostic span, and will still be sound. + pub(in super::super) fn normalize_associated_types_in_wf( + &self, + span: Span, + value: T, + hir_id: hir::HirId, + ) -> T + where + T: TypeFoldable<'tcx>, + { + self.inh.normalize_associated_types_in_with_cause( + ObligationCause::new(span, self.body_id, ObligationCauseCode::WellFormed(Some(hir_id))), + self.param_env, + value, + ) + } + pub(in super::super) fn normalize_associated_types_in(&self, span: Span, value: T) -> T where T: TypeFoldable<'tcx>, @@ -423,7 +446,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { where T: TypeFoldable<'tcx>, { - self.inh.partially_normalize_associated_types_in(span, self.body_id, self.param_env, value) + self.inh.partially_normalize_associated_types_in( + ObligationCause::misc(span, self.body_id), + self.param_env, + value, + ) } pub fn require_type_meets( diff --git a/compiler/rustc_typeck/src/check/inherited.rs b/compiler/rustc_typeck/src/check/inherited.rs index 2e9bef15f90..237861f1dd2 100644 --- a/compiler/rustc_typeck/src/check/inherited.rs +++ b/compiler/rustc_typeck/src/check/inherited.rs @@ -13,7 +13,7 @@ use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt}; use rustc_span::{self, Span}; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::opaque_types::OpaqueTypeDecl; -use rustc_trait_selection::traits::{self, TraitEngine, TraitEngineExt}; +use rustc_trait_selection::traits::{self, ObligationCause, TraitEngine, TraitEngineExt}; use std::cell::RefCell; use std::ops::Deref; @@ -162,7 +162,23 @@ impl Inherited<'a, 'tcx> { where T: TypeFoldable<'tcx>, { - let ok = self.partially_normalize_associated_types_in(span, body_id, param_env, value); + self.normalize_associated_types_in_with_cause( + ObligationCause::misc(span, body_id), + param_env, + value, + ) + } + + pub(super) fn normalize_associated_types_in_with_cause( + &self, + cause: ObligationCause<'tcx>, + param_env: ty::ParamEnv<'tcx>, + value: T, + ) -> T + where + T: TypeFoldable<'tcx>, + { + let ok = self.partially_normalize_associated_types_in(cause, param_env, value); self.register_infer_ok_obligations(ok) } } diff --git a/compiler/rustc_typeck/src/check/wfcheck.rs b/compiler/rustc_typeck/src/check/wfcheck.rs index 26d3cc9d891..bff391eb2d7 100644 --- a/compiler/rustc_typeck/src/check/wfcheck.rs +++ b/compiler/rustc_typeck/src/check/wfcheck.rs @@ -387,7 +387,7 @@ fn check_associated_item( ) { debug!("check_associated_item: {:?}", item_id); - let code = ObligationCauseCode::MiscObligation; + let code = ObligationCauseCode::WellFormed(Some(item_id)); for_id(tcx, item_id, span).with_fcx(|fcx| { let item = fcx.tcx.associated_item(fcx.tcx.hir().local_def_id(item_id)); @@ -401,7 +401,7 @@ fn check_associated_item( match item.kind { ty::AssocKind::Const => { let ty = fcx.tcx.type_of(item.def_id); - let ty = fcx.normalize_associated_types_in(span, ty); + let ty = fcx.normalize_associated_types_in_wf(span, ty, item_id); fcx.register_wf_obligation(ty.into(), span, code.clone()); } ty::AssocKind::Fn => { @@ -423,7 +423,7 @@ fn check_associated_item( } if item.defaultness.has_value() { let ty = fcx.tcx.type_of(item.def_id); - let ty = fcx.normalize_associated_types_in(span, ty); + let ty = fcx.normalize_associated_types_in_wf(span, ty, item_id); fcx.register_wf_obligation(ty.into(), span, code.clone()); } } @@ -515,7 +515,8 @@ fn check_type_defn<'tcx, F>( fcx.register_wf_obligation( field.ty.into(), field.span, - ObligationCauseCode::MiscObligation, + // We don't have an HIR id for the field + ObligationCauseCode::WellFormed(None), ) } @@ -621,7 +622,7 @@ fn check_item_type(tcx: TyCtxt<'_>, item_id: hir::HirId, ty_span: Span, allow_fo for_id(tcx, item_id, ty_span).with_fcx(|fcx| { let ty = tcx.type_of(tcx.hir().local_def_id(item_id)); - let item_ty = fcx.normalize_associated_types_in(ty_span, ty); + let item_ty = fcx.normalize_associated_types_in_wf(ty_span, ty, item_id); let mut forbid_unsized = true; if allow_foreign_ty { @@ -631,7 +632,11 @@ fn check_item_type(tcx: TyCtxt<'_>, item_id: hir::HirId, ty_span: Span, allow_fo } } - fcx.register_wf_obligation(item_ty.into(), ty_span, ObligationCauseCode::MiscObligation); + fcx.register_wf_obligation( + item_ty.into(), + ty_span, + ObligationCauseCode::WellFormed(Some(item_id)), + ); if forbid_unsized { fcx.register_bound( item_ty, @@ -680,7 +685,7 @@ fn check_impl<'tcx>( fcx.register_wf_obligation( self_ty.into(), ast_self_ty.span, - ObligationCauseCode::MiscObligation, + ObligationCauseCode::WellFormed(Some(item.hir_id())), ); } } @@ -746,7 +751,7 @@ fn check_where_clauses<'tcx, 'fcx>( fcx.register_wf_obligation( default_ct.into(), tcx.def_span(param.def_id), - ObligationCauseCode::MiscObligation, + ObligationCauseCode::WellFormed(None), ); } } @@ -900,7 +905,7 @@ fn check_fn_or_method<'fcx, 'tcx>( let sig = fcx.normalize_associated_types_in(span, sig); for (&input_ty, ty) in iter::zip(sig.inputs(), hir_decl.inputs) { - fcx.register_wf_obligation(input_ty.into(), ty.span, ObligationCauseCode::MiscObligation); + fcx.register_wf_obligation(input_ty.into(), ty.span, ObligationCauseCode::WellFormed(None)); } implied_bounds.extend(sig.inputs()); diff --git a/compiler/rustc_typeck/src/hir_wf_check.rs b/compiler/rustc_typeck/src/hir_wf_check.rs new file mode 100644 index 00000000000..fa9c44bb891 --- /dev/null +++ b/compiler/rustc_typeck/src/hir_wf_check.rs @@ -0,0 +1,133 @@ +use crate::collect::ItemCtxt; +use rustc_hir as hir; +use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; +use rustc_hir::HirId; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::ObligationCause; +use rustc_infer::traits::TraitEngine; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::{self, ToPredicate, TyCtxt}; +use rustc_trait_selection::traits; + +pub fn provide(providers: &mut Providers) { + *providers = Providers { diagnostic_hir_wf_check, ..*providers }; +} + +// Ideally, this would be in `rustc_trait_selection`, but we +// need access to `ItemCtxt` +fn diagnostic_hir_wf_check<'tcx>( + tcx: TyCtxt<'tcx>, + (predicate, hir_id): (ty::Predicate<'tcx>, HirId), +) -> Option> { + let hir = tcx.hir(); + // HIR wfcheck should only ever happen as part of improving an existing error + tcx.sess.delay_span_bug(hir.span(hir_id), "Performed HIR wfcheck without an existing error!"); + + // Currently, we only handle WF checking for items (e.g. associated items). + // It would be nice to extend this to handle wf checks inside functions. + let def_id = match tcx.hir().opt_local_def_id(hir_id) { + Some(def_id) => def_id, + None => return None, + }; + + // FIXME - figure out how we want to handle wf-checking for + // things inside a function body. + let icx = ItemCtxt::new(tcx, def_id.to_def_id()); + + // To perform HIR-based WF checking, we iterate over all HIR types + // that occur 'inside' the item we're checking. For example, + // given the type `Option>`, we will check + // `Option>`, `MyStruct`, and `u8`. + // For each type, we perform a well-formed check, and see if we get + // an erorr that matches our expected predicate. We keep save + // the `ObligationCause` corresponding to the *innermost* type, + // which is the most specific type that we can point to. + // In general, the different components of an `hir::Ty` may have + // completely differentr spans due to macro invocations. Pointing + // to the most accurate part of the type can be the difference + // between a useless span (e.g. the macro invocation site) + // and a useful span (e.g. a user-provided type passed in to the macro). + // + // This approach is quite inefficient - we redo a lot of work done + // by the normal WF checker. However, this code is run at most once + // per reported error - it will have no impact when compilation succeeds, + // and should only have an impact if a very large number of errors are + // displaydd to the user. + struct HirWfCheck<'tcx> { + tcx: TyCtxt<'tcx>, + predicate: ty::Predicate<'tcx>, + cause: Option>, + cause_depth: usize, + icx: ItemCtxt<'tcx>, + hir_id: HirId, + param_env: ty::ParamEnv<'tcx>, + depth: usize, + } + + impl<'tcx> Visitor<'tcx> for HirWfCheck<'tcx> { + type Map = intravisit::ErasedMap<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } + fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx>) { + self.tcx.infer_ctxt().enter(|infcx| { + let mut fulfill = traits::FulfillmentContext::new(); + let tcx_ty = self.icx.to_ty(ty); + let cause = traits::ObligationCause::new( + ty.span, + self.hir_id, + traits::ObligationCauseCode::MiscObligation, + ); + fulfill.register_predicate_obligation( + &infcx, + traits::Obligation::new( + cause, + self.param_env, + ty::PredicateKind::WellFormed(tcx_ty.into()).to_predicate(self.tcx), + ), + ); + + if let Err(errors) = fulfill.select_all_or_error(&infcx) { + tracing::debug!("Wf-check got errors for {:?}: {:?}", ty, errors); + for error in errors { + if error.obligation.predicate == self.predicate { + // Save the cause from the greatest depth - this corresponds + // to picking more-specific types (e.g. `MyStruct`) + // over less-specific types (e.g. `Option>`) + if self.depth >= self.cause_depth { + self.cause = Some(error.obligation.cause); + self.cause_depth = self.depth + } + } + } + } + }); + self.depth += 1; + intravisit::walk_ty(self, ty); + self.depth -= 1; + } + } + + let mut visitor = HirWfCheck { + tcx, + predicate, + cause: None, + cause_depth: 0, + icx, + hir_id, + param_env: tcx.param_env(def_id.to_def_id()), + depth: 0, + }; + + let ty = match tcx.hir().get(hir_id) { + hir::Node::ImplItem(item) => match item.kind { + hir::ImplItemKind::TyAlias(ref ty) => Some(ty), + _ => None, + }, + _ => None, + }; + if let Some(ty) = ty { + visitor.visit_ty(ty); + } + visitor.cause +} diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs index 92ef8297472..5b717862e02 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -88,6 +88,7 @@ mod coherence; mod collect; mod constrained_generic_params; mod errors; +pub mod hir_wf_check; mod impl_wf_check; mod mem_categorization; mod outlives; @@ -462,6 +463,7 @@ pub fn provide(providers: &mut Providers) { variance::provide(providers); outlives::provide(providers); impl_wf_check::provide(providers); + hir_wf_check::provide(providers); } pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorReported> { diff --git a/src/test/ui/wf/wf-complex-assoc-type.rs b/src/test/ui/wf/wf-complex-assoc-type.rs new file mode 100644 index 00000000000..c3811e82394 --- /dev/null +++ b/src/test/ui/wf/wf-complex-assoc-type.rs @@ -0,0 +1,12 @@ +trait MyTrait {} +struct AssertMyTrait(T); + +trait HelperTrait { + type MyItem; +} + +impl HelperTrait for () { + type MyItem = Option<((AssertMyTrait, u8))>; //~ ERROR the trait bound +} + +fn main() {} diff --git a/src/test/ui/wf/wf-complex-assoc-type.stderr b/src/test/ui/wf/wf-complex-assoc-type.stderr new file mode 100644 index 00000000000..c366519a8d7 --- /dev/null +++ b/src/test/ui/wf/wf-complex-assoc-type.stderr @@ -0,0 +1,12 @@ +error[E0277]: the trait bound `bool: MyTrait` is not satisfied + --> $DIR/wf-complex-assoc-type.rs:9:28 + | +LL | struct AssertMyTrait(T); + | ------- required by this bound in `AssertMyTrait` +... +LL | type MyItem = Option<((AssertMyTrait, u8))>; + | ^^^^^^^^^^^^^^^^^^^ the trait `MyTrait` is not implemented for `bool` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/wf/wf-impl-associated-type-trait.stderr b/src/test/ui/wf/wf-impl-associated-type-trait.stderr index 89399e30cad..0d4480fbf26 100644 --- a/src/test/ui/wf/wf-impl-associated-type-trait.stderr +++ b/src/test/ui/wf/wf-impl-associated-type-trait.stderr @@ -1,11 +1,11 @@ error[E0277]: the trait bound `T: MyHash` is not satisfied - --> $DIR/wf-impl-associated-type-trait.rs:17:5 + --> $DIR/wf-impl-associated-type-trait.rs:17:16 | LL | pub struct MySet { | ------ required by this bound in `MySet` ... LL | type Bar = MySet; - | ^^^^^^^^^^^^^^^^^^^^ the trait `MyHash` is not implemented for `T` + | ^^^^^^^^ the trait `MyHash` is not implemented for `T` | help: consider restricting type parameter `T` |