Skip to content

Commit

Permalink
Improve cause information for NLL higher-ranked errors
Browse files Browse the repository at this point in the history 
This PR has several interconnected pieces:

1. In some of the NLL region error code, we now pass
   around an `ObligationCause`, instead of just a plain `Span`.
   This gets forwarded into `fulfill_cx.register_predicate_obligation`
   during error reporting.
2. The general InferCtxt error reporting code is extended to
   handle `ObligationCauseCode::BindingObligation`
3. A new enum variant `ConstraintCategory::Predicate` is added.
   We try to avoid using this as the 'best blame constraint' - instead,
   we use it to enhance the `ObligationCause` of the `BlameConstraint`
   that we do end up choosing.

As a result, several NLL error messages now contain the same
"the lifetime requirement is introduced here" message as non-NLL
errors.

Having an `ObligationCause` available will likely prove useful
for future improvements to NLL error messages.
  • Loading branch information
@Aaron1011
Aaron1011 committed Sep 27, 2021
1 parent 3e8f32e commit 93ab12e
Show file tree
Hide file tree
Showing 16 changed files with 197 additions and 85 deletions.
118 changes: 73 additions & 45 deletions compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs
View file
Expand Up @@ -9,7 +9,7 @@ use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc_span::Span;
use rustc_trait_selection::traits::query::type_op;
use rustc_trait_selection::traits::{SelectionContext, TraitEngineExt as _};
use rustc_traits::{type_op_ascribe_user_type_with_span, type_op_prove_predicate_with_span};
use rustc_traits::{type_op_ascribe_user_type_with_span, type_op_prove_predicate_with_cause};

use std::fmt;
use std::rc::Rc;
Expand Down Expand Up @@ -45,21 +45,20 @@ impl UniverseInfo<'tcx> {
mbcx: &mut MirBorrowckCtxt<'_, 'tcx>,
placeholder: ty::PlaceholderRegion,
error_element: RegionElement,
span: Span,
cause: ObligationCause<'tcx>,
) {
match self.0 {
UniverseInfoInner::RelateTys { expected, found } => {
let body_id = mbcx.infcx.tcx.hir().local_def_id_to_hir_id(mbcx.mir_def_id());
let err = mbcx.infcx.report_mismatched_types(
&ObligationCause::misc(span, body_id),
&cause,
expected,
found,
TypeError::RegionsPlaceholderMismatch,
);
err.buffer(&mut mbcx.errors_buffer);
}
UniverseInfoInner::TypeOp(ref type_op_info) => {
type_op_info.report_error(mbcx, placeholder, error_element, span);
type_op_info.report_error(mbcx, placeholder, error_element, cause);
}
UniverseInfoInner::Other => {
// FIXME: This error message isn't great, but it doesn't show
Expand All @@ -68,7 +67,7 @@ impl UniverseInfo<'tcx> {
mbcx.infcx
.tcx
.sess
.struct_span_err(span, "higher-ranked subtype error")
.struct_span_err(cause.span, "higher-ranked subtype error")
.buffer(&mut mbcx.errors_buffer);
}
}
Expand Down Expand Up @@ -130,7 +129,7 @@ trait TypeOpInfo<'tcx> {
fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
span: Span,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>>;
Expand All @@ -140,7 +139,7 @@ trait TypeOpInfo<'tcx> {
mbcx: &mut MirBorrowckCtxt<'_, 'tcx>,
placeholder: ty::PlaceholderRegion,
error_element: RegionElement,
span: Span,
cause: ObligationCause<'tcx>,
) {
let tcx = mbcx.infcx.tcx;
let base_universe = self.base_universe();
Expand All @@ -150,7 +149,7 @@ trait TypeOpInfo<'tcx> {
{
adjusted
} else {
self.fallback_error(tcx, span).buffer(&mut mbcx.errors_buffer);
self.fallback_error(tcx, cause.span).buffer(&mut mbcx.errors_buffer);
return;
};

Expand All @@ -175,7 +174,8 @@ trait TypeOpInfo<'tcx> {

debug!(?placeholder_region);

let nice_error = self.nice_error(tcx, span, placeholder_region, error_region);
let span = cause.span;
let nice_error = self.nice_error(tcx, cause, placeholder_region, error_region);

if let Some(nice_error) = nice_error {
nice_error.buffer(&mut mbcx.errors_buffer);
Expand Down Expand Up @@ -205,15 +205,24 @@ impl TypeOpInfo<'tcx> for PredicateQuery<'tcx> {
fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
span: Span,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
tcx.infer_ctxt().enter_with_canonical(span, &self.canonical_query, |ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);
type_op_prove_predicate_with_span(infcx, &mut *fulfill_cx, key, Some(span));
try_extract_error_from_fulfill_cx(fulfill_cx, infcx, placeholder_region, error_region)
})
tcx.infer_ctxt().enter_with_canonical(
cause.span,
&self.canonical_query,
|ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);
type_op_prove_predicate_with_cause(infcx, &mut *fulfill_cx, key, cause);
try_extract_error_from_fulfill_cx(
fulfill_cx,
infcx,
placeholder_region,
error_region,
)
},
)
}
}

Expand All @@ -239,32 +248,41 @@ where
fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
span: Span,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
tcx.infer_ctxt().enter_with_canonical(span, &self.canonical_query, |ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);

let mut selcx = SelectionContext::new(infcx);

// FIXME(lqd): Unify and de-duplicate the following with the actual
// `rustc_traits::type_op::type_op_normalize` query to allow the span we need in the
// `ObligationCause`. The normalization results are currently different between
// `AtExt::normalize` used in the query and `normalize` called below: the former fails
// to normalize the `nll/relate_tys/impl-fn-ignore-binder-via-bottom.rs` test. Check
// after #85499 lands to see if its fixes have erased this difference.
let (param_env, value) = key.into_parts();
let Normalized { value: _, obligations } = rustc_trait_selection::traits::normalize(
&mut selcx,
param_env,
ObligationCause::dummy_with_span(span),
value.value,
);
fulfill_cx.register_predicate_obligations(infcx, obligations);

try_extract_error_from_fulfill_cx(fulfill_cx, infcx, placeholder_region, error_region)
})
tcx.infer_ctxt().enter_with_canonical(
cause.span,
&self.canonical_query,
|ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);

let mut selcx = SelectionContext::new(infcx);

// FIXME(lqd): Unify and de-duplicate the following with the actual
// `rustc_traits::type_op::type_op_normalize` query to allow the span we need in the
// `ObligationCause`. The normalization results are currently different between
// `AtExt::normalize` used in the query and `normalize` called below: the former fails
// to normalize the `nll/relate_tys/impl-fn-ignore-binder-via-bottom.rs` test. Check
// after #85499 lands to see if its fixes have erased this difference.
let (param_env, value) = key.into_parts();
let Normalized { value: _, obligations } = rustc_trait_selection::traits::normalize(
&mut selcx,
param_env,
cause,
value.value,
);
fulfill_cx.register_predicate_obligations(infcx, obligations);

try_extract_error_from_fulfill_cx(
fulfill_cx,
infcx,
placeholder_region,
error_region,
)
},
)
}
}

Expand All @@ -287,15 +305,25 @@ impl TypeOpInfo<'tcx> for AscribeUserTypeQuery<'tcx> {
fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
span: Span,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
tcx.infer_ctxt().enter_with_canonical(span, &self.canonical_query, |ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);
type_op_ascribe_user_type_with_span(infcx, &mut *fulfill_cx, key, Some(span)).ok()?;
try_extract_error_from_fulfill_cx(fulfill_cx, infcx, placeholder_region, error_region)
})
tcx.infer_ctxt().enter_with_canonical(
cause.span,
&self.canonical_query,
|ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);
type_op_ascribe_user_type_with_span(infcx, &mut *fulfill_cx, key, Some(cause.span))
.ok()?;
try_extract_error_from_fulfill_cx(
fulfill_cx,
infcx,
placeholder_region,
error_region,
)
},
)
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file
Expand Up @@ -300,7 +300,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
borrow_region: RegionVid,
outlived_region: RegionVid,
) -> (ConstraintCategory, bool, Span, Option<RegionName>) {
let BlameConstraint { category, from_closure, span, variance_info: _ } =
let BlameConstraint { category, from_closure, cause, variance_info: _ } =
self.regioncx.best_blame_constraint(
&self.body,
borrow_region,
Expand All @@ -310,7 +310,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

let outlived_fr_name = self.give_region_a_name(outlived_region);

(category, from_closure, span, outlived_fr_name)
(category, from_closure, cause.span, outlived_fr_name)
}

/// Returns structured explanation for *why* the borrow contains the
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View file
Expand Up @@ -13,6 +13,7 @@ use rustc_span::{BytePos, Span};

use crate::borrowck_errors;

use super::{OutlivesSuggestionBuilder, RegionName};
use crate::region_infer::BlameConstraint;
use crate::{
nll::ConstraintDescription,
Expand All @@ -21,8 +22,6 @@ use crate::{
MirBorrowckCtxt,
};

use super::{OutlivesSuggestionBuilder, RegionName};

impl ConstraintDescription for ConstraintCategory {
fn description(&self) -> &'static str {
// Must end with a space. Allows for empty names to be provided.
Expand All @@ -41,7 +40,8 @@ impl ConstraintDescription for ConstraintCategory {
ConstraintCategory::OpaqueType => "opaque type ",
ConstraintCategory::ClosureUpvar(_) => "closure capture ",
ConstraintCategory::Usage => "this usage ",
ConstraintCategory::Boring
ConstraintCategory::Predicate(_, _)
| ConstraintCategory::Boring
| ConstraintCategory::BoringNoLocation
| ConstraintCategory::Internal => "",
}
Expand Down Expand Up @@ -217,7 +217,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let error_vid = self.regioncx.region_from_element(longer_fr, &error_element);

// Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
let (_, span) = self.regioncx.find_outlives_blame_span(
let (_, cause) = self.regioncx.find_outlives_blame_span(
&self.body,
longer_fr,
NllRegionVariableOrigin::Placeholder(placeholder),
Expand All @@ -227,7 +227,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let universe = placeholder.universe;
let universe_info = self.regioncx.universe_info(universe);

universe_info.report_error(self, placeholder, error_element, span);
universe_info.report_error(self, placeholder, error_element, cause);
}

RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => {
Expand Down Expand Up @@ -275,15 +275,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
) {
debug!("report_region_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);

let BlameConstraint { category, span, variance_info, from_closure: _ } =
let BlameConstraint { category, cause, variance_info, from_closure: _ } =
self.regioncx.best_blame_constraint(&self.body, fr, fr_origin, |r| {
self.regioncx.provides_universal_region(r, fr, outlived_fr)
});

debug!("report_region_error: category={:?} {:?} {:?}", category, span, variance_info);
debug!("report_region_error: category={:?} {:?} {:?}", category, cause, variance_info);
// Check if we can use one of the "nice region errors".
if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) {
let nice = NiceRegionError::new_from_span(self.infcx, span, o, f);
let nice = NiceRegionError::new_from_span(self.infcx, cause.span, o, f);
if let Some(diag) = nice.try_report_from_nll() {
diag.buffer(&mut self.errors_buffer);
return;
Expand All @@ -306,7 +306,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
fr_is_local,
outlived_fr_is_local,
category,
span,
span: cause.span,
};

let mut diag = match (category, fr_is_local, outlived_fr_is_local) {
Expand Down

0 comments on commit 93ab12e

Please sign in to comment.