Skip to content

Commit

Permalink
Improve opaque type higher-ranked region error message under NLL
Browse files Browse the repository at this point in the history
Currently, any higher-ranked region errors involving opaque types
fall back to a generic "higher-ranked subtype error" message when
run under NLL. This PR adds better error message handling for this
case, giving us the same kinds of error messages that we currently
get without NLL:

```
error: implementation of `MyTrait` is not general enough
  --> $DIR/opaque-hrtb.rs:12:13
   |
LL | fn foo() -> impl for<'a> MyTrait<&'a str> {
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MyTrait` is not general enough
   |
   = note: `impl MyTrait<&'2 str>` must implement `MyTrait<&'1 str>`, for any lifetime `'1`...
   = note: ...but it actually implements `MyTrait<&'2 str>`, for some specific lifetime `'2`

error: aborting due to previous error
```

To accomplish this, several different refactoring needed to be made:

* We now have a dedicated `InstantiateOpaqueType` struct which
implements `TypeOp`. This is used to invoke `instantiate_opaque_types`
during MIR type checking.
* `TypeOp` is refactored to pass around a `MirBorrowckCtxt`, which is
needed to report opaque type region errors.
* We no longer assume that all `TypeOp`s correspond to canonicalized
queries. This allows us to properly handle opaque type instantiation
(which does not occur in a query) as a `TypeOp`.
A new `ErrorInfo` associated type is used to determine what
additional information is used during higher-ranked region error
handling.
* The body of `try_extract_error_from_fulfill_cx`
has been moved out to a new function `try_extract_error_from_region_constraints`.
This allows us to re-use the same error reporting code between
canonicalized queries (which can extract region constraints directly
from a fresh `InferCtxt`) and opaque type handling (which needs to take
region constraints from the pre-existing `InferCtxt` that we use
throughout MIR borrow checking).
  • Loading branch information
Aaron1011 committed Feb 8, 2022
1 parent e7cc3bd commit 48a48fd
Show file tree
Hide file tree
Showing 14 changed files with 231 additions and 71 deletions.
108 changes: 87 additions & 21 deletions compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs
Expand Up @@ -2,9 +2,13 @@ use rustc_errors::DiagnosticBuilder;
use rustc_infer::infer::canonical::Canonical;
use rustc_infer::infer::error_reporting::nice_region_error::NiceRegionError;
use rustc_infer::infer::region_constraints::Constraint;
use rustc_infer::infer::region_constraints::RegionConstraintData;
use rustc_infer::infer::RegionVariableOrigin;
use rustc_infer::infer::{InferCtxt, RegionResolutionError, SubregionOrigin, TyCtxtInferExt as _};
use rustc_infer::traits::{Normalized, ObligationCause, TraitEngine, TraitEngineExt};
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::RegionVid;
use rustc_middle::ty::UniverseIndex;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc_span::Span;
use rustc_trait_selection::traits::query::type_op;
Expand Down Expand Up @@ -78,6 +82,15 @@ crate trait ToUniverseInfo<'tcx> {
fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx>;
}

impl<'tcx> ToUniverseInfo<'tcx> for crate::type_check::InstantiateOpaqueType<'tcx> {
fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(crate::type_check::InstantiateOpaqueType {
base_universe: Some(base_universe),
..self
})))
}
}

impl<'tcx> ToUniverseInfo<'tcx>
for Canonical<'tcx, ty::ParamEnvAnd<'tcx, type_op::prove_predicate::ProvePredicate<'tcx>>>
{
Expand Down Expand Up @@ -118,6 +131,12 @@ impl<'tcx, F, G> ToUniverseInfo<'tcx> for Canonical<'tcx, type_op::custom::Custo
}
}

impl<'tcx> ToUniverseInfo<'tcx> for ! {
fn to_universe_info(self, _base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
self
}
}

#[allow(unused_lifetimes)]
trait TypeOpInfo<'tcx> {
/// Returns an error to be reported if rerunning the type op fails to
Expand All @@ -128,7 +147,7 @@ trait TypeOpInfo<'tcx> {

fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
mbcx: &mut MirBorrowckCtxt<'_, 'tcx>,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
Expand Down Expand Up @@ -175,7 +194,7 @@ trait TypeOpInfo<'tcx> {
debug!(?placeholder_region);

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

if let Some(nice_error) = nice_error {
nice_error.buffer(&mut mbcx.errors_buffer);
Expand Down Expand Up @@ -204,16 +223,16 @@ impl<'tcx> TypeOpInfo<'tcx> for PredicateQuery<'tcx> {

fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
mbcx: &mut MirBorrowckCtxt<'_, 'tcx>,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
tcx.infer_ctxt().enter_with_canonical(
mbcx.infcx.tcx.infer_ctxt().enter_with_canonical(
cause.span,
&self.canonical_query,
|ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx);
type_op_prove_predicate_with_cause(infcx, &mut *fulfill_cx, key, cause);
try_extract_error_from_fulfill_cx(
fulfill_cx,
Expand Down Expand Up @@ -247,16 +266,16 @@ where

fn nice_error(
&self,
tcx: TyCtxt<'tcx>,
mbcx: &mut MirBorrowckCtxt<'_, 'tcx>,
cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
tcx.infer_ctxt().enter_with_canonical(
mbcx.infcx.tcx.infer_ctxt().enter_with_canonical(
cause.span,
&self.canonical_query,
|ref infcx, key, _| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(tcx);
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx);

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

Expand Down Expand Up @@ -304,16 +323,16 @@ impl<'tcx> TypeOpInfo<'tcx> for AscribeUserTypeQuery<'tcx> {

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

impl<'tcx> TypeOpInfo<'tcx> for crate::type_check::InstantiateOpaqueType<'tcx> {
fn fallback_error(&self, tcx: TyCtxt<'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
// FIXME: This error message isn't great, but it doesn't show up in the existing UI tests,
// and is only the fallback when the nice error fails. Consider improving this some more.
tcx.sess.struct_span_err(span, "higher-ranked lifetime error for opaque type!")
}

fn base_universe(&self) -> ty::UniverseIndex {
self.base_universe.unwrap()
}

fn nice_error(
&self,
mbcx: &mut MirBorrowckCtxt<'_, 'tcx>,
_cause: ObligationCause<'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
try_extract_error_from_region_constraints(
mbcx.infcx,
placeholder_region,
error_region,
self.region_constraints.as_ref().unwrap(),
// We're using the original `InferCtxt` that we
// started MIR borrowchecking with, so the region
// constraints have already been taken. Use the data from
// our `mbcx` instead.
|vid| mbcx.regioncx.var_infos[vid].origin,
|vid| mbcx.regioncx.var_infos[vid].universe,
)
}
}

#[instrument(skip(fulfill_cx, infcx), level = "debug")]
fn try_extract_error_from_fulfill_cx<'tcx>(
mut fulfill_cx: Box<dyn TraitEngine<'tcx> + 'tcx>,
infcx: &InferCtxt<'_, 'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
) -> Option<DiagnosticBuilder<'tcx>> {
let tcx = infcx.tcx;

// We generally shouldn't have errors here because the query was
// already run, but there's no point using `delay_span_bug`
// when we're going to emit an error here anyway.
let _errors = fulfill_cx.select_all_or_error(infcx);
let region_constraints = infcx.with_region_constraints(|r| r.clone());
try_extract_error_from_region_constraints(
infcx,
placeholder_region,
error_region,
&region_constraints,
|vid| infcx.region_var_origin(vid),
|vid| infcx.universe_of_region(infcx.tcx.mk_region(ty::ReVar(vid))),
)
}

let (sub_region, cause) = infcx.with_region_constraints(|region_constraints| {
debug!("{:#?}", region_constraints);
fn try_extract_error_from_region_constraints<'tcx>(
infcx: &InferCtxt<'_, 'tcx>,
placeholder_region: ty::Region<'tcx>,
error_region: Option<ty::Region<'tcx>>,
region_constraints: &RegionConstraintData<'tcx>,
mut region_var_origin: impl FnMut(RegionVid) -> RegionVariableOrigin,
mut universe_of_region: impl FnMut(RegionVid) -> UniverseIndex,
) -> Option<DiagnosticBuilder<'tcx>> {
let (sub_region, cause) =
region_constraints.constraints.iter().find_map(|(constraint, cause)| {
match *constraint {
Constraint::RegSubReg(sub, sup) if sup == placeholder_region && sup != sub => {
Some((sub, cause.clone()))
}
// FIXME: Should this check the universe of the var?
Constraint::VarSubReg(vid, sup) if sup == placeholder_region => {
Some((tcx.mk_region(ty::ReVar(vid)), cause.clone()))
Some((infcx.tcx.mk_region(ty::ReVar(vid)), cause.clone()))
}
_ => None,
}
})
})?;
})?;

debug!(?sub_region, "cause = {:#?}", cause);
let nice_error = match (error_region, sub_region) {
(Some(error_region), &ty::ReVar(vid)) => NiceRegionError::new(
infcx,
RegionResolutionError::SubSupConflict(
vid,
infcx.region_var_origin(vid),
region_var_origin(vid),
cause.clone(),
error_region,
cause.clone(),
Expand All @@ -380,8 +446,8 @@ fn try_extract_error_from_fulfill_cx<'tcx>(
infcx,
RegionResolutionError::UpperBoundUniverseConflict(
vid,
infcx.region_var_origin(vid),
infcx.universe_of_region(sub_region),
region_var_origin(vid),
universe_of_region(vid),
cause.clone(),
placeholder_region,
),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/lib.rs
Expand Up @@ -5,6 +5,7 @@
#![feature(crate_visibility_modifier)]
#![feature(let_else)]
#![feature(min_specialization)]
#![feature(never_type)]
#![feature(stmt_expr_attributes)]
#![feature(trusted_step)]
#![feature(try_blocks)]
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_borrowck/src/region_infer/mod.rs
Expand Up @@ -44,6 +44,7 @@ mod reverse_sccs;
pub mod values;

pub struct RegionInferenceContext<'tcx> {
pub var_infos: VarInfos,
/// Contains the definition for every region variable. Region
/// variables are identified by their index (`RegionVid`). The
/// definition contains information about where the region came
Expand Down Expand Up @@ -266,7 +267,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
) -> Self {
// Create a RegionDefinition for each inference variable.
let definitions: IndexVec<_, _> = var_infos
.into_iter()
.iter()
.map(|info| RegionDefinition::new(info.universe, info.origin))
.collect();

Expand All @@ -291,6 +292,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
Rc::new(member_constraints_in.into_mapped(|r| constraint_sccs.scc(r)));

let mut result = Self {
var_infos,
definitions,
liveness_constraints,
constraints,
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Expand Up @@ -33,12 +33,11 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
) -> Fallible<R>
where
Op: type_op::TypeOp<'tcx, Output = R>,
Canonical<'tcx, Op>: ToUniverseInfo<'tcx>,
Op::ErrorInfo: ToUniverseInfo<'tcx>,
{
let old_universe = self.infcx.universe();

let TypeOpOutput { output, constraints, canonicalized_query } =
op.fully_perform(self.infcx)?;
let TypeOpOutput { output, constraints, error_info } = op.fully_perform(self.infcx)?;

if let Some(data) = &constraints {
self.push_region_constraints(locations, category, data);
Expand All @@ -47,8 +46,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
let universe = self.infcx.universe();

if old_universe != universe {
let universe_info = match canonicalized_query {
Some(canonicalized_query) => canonicalized_query.to_universe_info(old_universe),
let universe_info = match error_info {
Some(error_info) => error_info.to_universe_info(old_universe),
None => UniverseInfo::other(),
};
for u in old_universe..universe {
Expand Down
Expand Up @@ -268,7 +268,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
TypeOpOutput {
output: self.infcx.tcx.ty_error(),
constraints: None,
canonicalized_query: None,
error_info: None,
}
});
// Note: we need this in examples like
Expand Down
34 changes: 33 additions & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Expand Up @@ -17,6 +17,7 @@ use rustc_hir::lang_items::LangItem;
use rustc_index::vec::{Idx, IndexVec};
use rustc_infer::infer::canonical::QueryRegionConstraints;
use rustc_infer::infer::outlives::env::RegionBoundPairs;
use rustc_infer::infer::region_constraints::RegionConstraintData;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{
InferCtxt, InferOk, LateBoundRegionConversionTime, NllRegionVariableOrigin,
Expand All @@ -39,9 +40,11 @@ use rustc_target::abi::VariantIdx;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
use rustc_trait_selection::traits::query::type_op;
use rustc_trait_selection::traits::query::type_op::custom::scrape_region_constraints;
use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp;
use rustc_trait_selection::traits::query::type_op::{TypeOp, TypeOpOutput};
use rustc_trait_selection::traits::query::Fallible;
use rustc_trait_selection::traits::{self, ObligationCause};
use rustc_trait_selection::traits::{self, ObligationCause, PredicateObligation};

use rustc_const_eval::transform::{
check_consts::ConstCx, promote_consts::is_const_fn_in_array_repeat_expression,
Expand Down Expand Up @@ -2637,3 +2640,32 @@ impl NormalizeLocation for Location {
Locations::Single(self)
}
}

/// Runs `infcx.instantiate_opaque_types`. Unlike other `TypeOp`s,
/// this is not canonicalized - it directly affects the main `InferCtxt`
/// that we use during MIR borrowchecking.
#[derive(Debug)]
pub(super) struct InstantiateOpaqueType<'tcx> {
pub base_universe: Option<ty::UniverseIndex>,
pub region_constraints: Option<RegionConstraintData<'tcx>>,
pub obligation: PredicateObligation<'tcx>,
}

impl<'tcx> TypeOp<'tcx> for InstantiateOpaqueType<'tcx> {
type Output = ();
/// We use this type itself to store the information used
/// when reporting errors. Since this is not a query, we don't
/// re-run anything during error reporting - we just use the information
/// we saved to help extract an error from the already-existing region
/// constraints in our `InferCtxt`
type ErrorInfo = InstantiateOpaqueType<'tcx>;

fn fully_perform(mut self, infcx: &InferCtxt<'_, 'tcx>) -> Fallible<TypeOpOutput<'tcx, Self>> {
let (mut output, region_constraints) = scrape_region_constraints(infcx, || {
Ok(InferOk { value: (), obligations: vec![self.obligation.clone()] })
})?;
self.region_constraints = Some(region_constraints);
output.error_info = Some(self);
Ok(output)
}
}

0 comments on commit 48a48fd

Please sign in to comment.