bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Auto merge of #83898 - Aaron1011:feature/hir-wf, r=estebank

Browse source 
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<MyStruct<u8>>`
will cause us to check `MyStruct<u8>` 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<MyStruct<u8>>;` 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).
This commit is contained in:
bors 2021-07-16 21:54:42 +00:00
commit 32c447e179
23 changed files with 350 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

7
compiler/rustc_trait_selection/src/infer.rs
View file

@ -30,8 +30,7 @@ pub trait InferCtxtExt<'tcx> {
fn partially_normalize_associated_types_in<T>(
&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<T>(
&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!(

9
compiler/rustc_trait_selection/src/opaque_types.rs
View file

@ -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;
@ -1049,8 +1049,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);

13
compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs
View file

@ -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,
}),
}
}

34
compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file

@ -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,

3
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file

@ -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");
}

8
compiler/rustc_trait_selection/src/traits/fulfill.rs
View file

@ -717,6 +717,10 @@ fn substs_infer_vars<'a, 'tcx>(
fn to_fulfillment_error<'tcx>(
error: Error<PendingPredicateObligation<'tcx>, 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)
}

4
compiler/rustc_trait_selection/src/traits/select/mod.rs
View file

@ -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)

4
compiler/rustc_trait_selection/src/traits/wf.rs
View file

@ -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);