Skip to content

Commit

Permalink
Auto merge of #85343 - Aaron1011:variance-diag, r=estebank
Browse files Browse the repository at this point in the history
Add variance-related information to lifetime error messages

This PR adds a basic framework for displaying variance-related information in error messages. For example:

```
error: lifetime may not live long enough
  --> $DIR/type-check-pointer-comparisons.rs:12:5
   |
LL | fn compare_mut<'a, 'b>(x: *mut &'a i32, y: *mut &'b i32) {
   |                --  -- lifetime `'b` defined here
   |                |
   |                lifetime `'a` defined here
LL |     x == y;
   |     ^ requires that `'a` must outlive `'b`
   |
   = help: consider adding the following bound: `'a: 'b`
   = note: requirement occurs because of a mutable pointer to &i32
   = note: mutable pointers are invariant over their type parameter
   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
```

The last three lines are new.

This is accomplished by adding a new struct `VarianceDiagInfo`, and passing it along through the various relation methods. When relating types that change the variance (e.g. `&mut T` or `*mut T`), we pass a more specific `VarianceDiagInfo` storing information about the cause of the variance change. When an error, we use the `VarianceDiagInfo` to add additional information to the error message.

This PR doesn't change any variance-related computation or behavior - only diagnostic messages. Therefore, the implementation is quite incomplete - more detailed error messages can be filled in in subsequent PRs.

Limitations:
* We only attempt to deal with invariance - since it's at the bottom of the 'variance lattice', our variance will never change again after it becomes invariant. Handling contravariance would be trickier, since we can change between contravariance and covariance multiple times (e.g. `fn(fn(&'static u8))`). Since contravariance (AFAIK) is only used for function arguments, we can probably get away without a very fancy message for cases involving contravariance.
* `VarianceDiagInfo` currently only handles mutable pointers/references. However, user-defined types (structs, enums, and unions) have the variance of their type parameters inferred, so it would be good to eventually display information about that. We'll want to try to find a balance between displaying too much and too little information about how the variance was inferred.
* The improved error messages are only displayed when `#![feature(nll)]` / `-Z borrowck=mir` is enabled.  If issue #58781 is not resolved relatively soon, then we might want to duplicate some of this logic in the 'current' (non-NLL) region/outlives handling code.
  • Loading branch information
bors committed Jun 6, 2021
2 parents 5b638c1 + fad2242 commit 35fff69
Show file tree
Hide file tree
Showing 33 changed files with 397 additions and 144 deletions.
7 changes: 6 additions & 1 deletion compiler/rustc_infer/src/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,12 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for QueryTypeRelatingDelegate<'_, 'tcx> {
)
}

fn push_outlives(&mut self, sup: ty::Region<'tcx>, sub: ty::Region<'tcx>) {
fn push_outlives(
&mut self,
sup: ty::Region<'tcx>,
sub: ty::Region<'tcx>,
_info: ty::VarianceDiagInfo<'tcx>,
) {
self.obligations.push(Obligation {
cause: self.cause.clone(),
param_env: self.param_env,
Expand Down
25 changes: 20 additions & 5 deletions compiler/rustc_infer/src/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,12 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
match dir {
EqTo => self.equate(a_is_expected).relate(a_ty, b_ty),
SubtypeOf => self.sub(a_is_expected).relate(a_ty, b_ty),
SupertypeOf => {
self.sub(a_is_expected).relate_with_variance(ty::Contravariant, a_ty, b_ty)
}
SupertypeOf => self.sub(a_is_expected).relate_with_variance(
ty::Contravariant,
ty::VarianceDiagInfo::default(),
a_ty,
b_ty,
),
}?;

Ok(())
Expand Down Expand Up @@ -574,6 +577,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down Expand Up @@ -737,7 +741,12 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
if self.tcx().lazy_normalization() =>
{
assert_eq!(promoted, None);
let substs = self.relate_with_variance(ty::Variance::Invariant, substs, substs)?;
let substs = self.relate_with_variance(
ty::Variance::Invariant,
ty::VarianceDiagInfo::default(),
substs,
substs,
)?;
Ok(self.tcx().mk_const(ty::Const {
ty: c.ty,
val: ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted }),
Expand Down Expand Up @@ -831,6 +840,7 @@ impl TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
_variance: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down Expand Up @@ -965,7 +975,12 @@ impl TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> {
if self.tcx().lazy_normalization() =>
{
assert_eq!(promoted, None);
let substs = self.relate_with_variance(ty::Variance::Invariant, substs, substs)?;
let substs = self.relate_with_variance(
ty::Variance::Invariant,
ty::VarianceDiagInfo::default(),
substs,
substs,
)?;
Ok(self.tcx().mk_const(ty::Const {
ty: c.ty,
val: ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted }),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/equate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl TypeRelation<'tcx> for Equate<'combine, 'infcx, 'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
_: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_infer/src/infer/glb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ impl TypeRelation<'tcx> for Glb<'combine, 'infcx, 'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down Expand Up @@ -96,7 +97,7 @@ impl TypeRelation<'tcx> for Glb<'combine, 'infcx, 'tcx> {
// When higher-ranked types are involved, computing the LUB is
// very challenging, switch to invariance. This is obviously
// overly conservative but works ok in practice.
self.relate_with_variance(ty::Variance::Invariant, a, b)?;
self.relate_with_variance(ty::Variance::Invariant, ty::VarianceDiagInfo::default(), a, b)?;
Ok(a)
}
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_infer/src/infer/lub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ impl TypeRelation<'tcx> for Lub<'combine, 'infcx, 'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down Expand Up @@ -96,7 +97,7 @@ impl TypeRelation<'tcx> for Lub<'combine, 'infcx, 'tcx> {
// When higher-ranked types are involved, computing the LUB is
// very challenging, switch to invariance. This is obviously
// overly conservative but works ok in practice.
self.relate_with_variance(ty::Variance::Invariant, a, b)?;
self.relate_with_variance(ty::Variance::Invariant, ty::VarianceDiagInfo::default(), a, b)?;
Ok(a)
}
}
Expand Down
34 changes: 28 additions & 6 deletions compiler/rustc_infer/src/infer/nll_relate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ where
/// - Bivariant means that it doesn't matter.
ambient_variance: ty::Variance,

ambient_variance_info: ty::VarianceDiagInfo<'tcx>,

/// When we pass through a set of binders (e.g., when looking into
/// a `fn` type), we push a new bound region scope onto here. This
/// will contain the instantiated region for each region in those
Expand All @@ -78,7 +80,12 @@ pub trait TypeRelatingDelegate<'tcx> {
/// satisfied for the two types to be related. `sub` and `sup` may
/// be regions from the type or new variables created through the
/// delegate.
fn push_outlives(&mut self, sup: ty::Region<'tcx>, sub: ty::Region<'tcx>);
fn push_outlives(
&mut self,
sup: ty::Region<'tcx>,
sub: ty::Region<'tcx>,
info: ty::VarianceDiagInfo<'tcx>,
);

fn const_equate(&mut self, a: &'tcx ty::Const<'tcx>, b: &'tcx ty::Const<'tcx>);

Expand Down Expand Up @@ -138,7 +145,14 @@ where
delegate: D,
ambient_variance: ty::Variance,
) -> Self {
Self { infcx, delegate, ambient_variance, a_scopes: vec![], b_scopes: vec![] }
Self {
infcx,
delegate,
ambient_variance,
ambient_variance_info: ty::VarianceDiagInfo::default(),
a_scopes: vec![],
b_scopes: vec![],
}
}

fn ambient_covariance(&self) -> bool {
Expand Down Expand Up @@ -239,10 +253,15 @@ where

/// Push a new outlives requirement into our output set of
/// constraints.
fn push_outlives(&mut self, sup: ty::Region<'tcx>, sub: ty::Region<'tcx>) {
fn push_outlives(
&mut self,
sup: ty::Region<'tcx>,
sub: ty::Region<'tcx>,
info: ty::VarianceDiagInfo<'tcx>,
) {
debug!("push_outlives({:?}: {:?})", sup, sub);

self.delegate.push_outlives(sup, sub);
self.delegate.push_outlives(sup, sub, info);
}

/// Relate a projection type and some value type lazily. This will always
Expand Down Expand Up @@ -490,13 +509,15 @@ where
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
debug!("relate_with_variance(variance={:?}, a={:?}, b={:?})", variance, a, b);

let old_ambient_variance = self.ambient_variance;
self.ambient_variance = self.ambient_variance.xform(variance);
self.ambient_variance_info = self.ambient_variance_info.clone().xform(info);

debug!("relate_with_variance: ambient_variance = {:?}", self.ambient_variance);

Expand Down Expand Up @@ -574,12 +595,12 @@ where

if self.ambient_covariance() {
// Covariance: a <= b. Hence, `b: a`.
self.push_outlives(v_b, v_a);
self.push_outlives(v_b, v_a, self.ambient_variance_info.clone());
}

if self.ambient_contravariance() {
// Contravariant: b <= a. Hence, `a: b`.
self.push_outlives(v_a, v_b);
self.push_outlives(v_a, v_b, self.ambient_variance_info.clone());
}

Ok(a)
Expand Down Expand Up @@ -835,6 +856,7 @@ where
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/sub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl TypeRelation<'tcx> for Sub<'combine, 'infcx, 'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
_info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ impl TypeRelation<'tcx> for Match<'tcx> {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
_: ty::Variance,
_: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T> {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub use self::sty::{
ExistentialPredicate, ExistentialProjection, ExistentialTraitRef, FnSig, FreeRegion, GenSig,
GeneratorSubsts, GeneratorSubstsParts, ParamConst, ParamTy, PolyExistentialProjection,
PolyExistentialTraitRef, PolyFnSig, PolyGenSig, PolyTraitRef, ProjectionTy, Region, RegionKind,
RegionVid, TraitRef, TyKind, TypeAndMut, UpvarSubsts,
RegionVid, TraitRef, TyKind, TypeAndMut, UpvarSubsts, VarianceDiagInfo, VarianceDiagMutKind,
};
pub use self::trait_def::TraitDef;

Expand Down
85 changes: 57 additions & 28 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub trait TypeRelation<'tcx>: Sized {
fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
info: ty::VarianceDiagInfo<'tcx>,
a: T,
b: T,
) -> RelateResult<'tcx, T>;
Expand Down Expand Up @@ -111,24 +112,23 @@ pub trait Relate<'tcx>: TypeFoldable<'tcx> + Copy {
///////////////////////////////////////////////////////////////////////////
// Relate impls

impl<'tcx> Relate<'tcx> for ty::TypeAndMut<'tcx> {
fn relate<R: TypeRelation<'tcx>>(
relation: &mut R,
a: ty::TypeAndMut<'tcx>,
b: ty::TypeAndMut<'tcx>,
) -> RelateResult<'tcx, ty::TypeAndMut<'tcx>> {
debug!("{}.mts({:?}, {:?})", relation.tag(), a, b);
if a.mutbl != b.mutbl {
Err(TypeError::Mutability)
} else {
let mutbl = a.mutbl;
let variance = match mutbl {
ast::Mutability::Not => ty::Covariant,
ast::Mutability::Mut => ty::Invariant,
};
let ty = relation.relate_with_variance(variance, a.ty, b.ty)?;
Ok(ty::TypeAndMut { ty, mutbl })
}
fn relate_type_and_mut<'tcx, R: TypeRelation<'tcx>>(
relation: &mut R,
a: ty::TypeAndMut<'tcx>,
b: ty::TypeAndMut<'tcx>,
kind: ty::VarianceDiagMutKind,
) -> RelateResult<'tcx, ty::TypeAndMut<'tcx>> {
debug!("{}.mts({:?}, {:?})", relation.tag(), a, b);
if a.mutbl != b.mutbl {
Err(TypeError::Mutability)
} else {
let mutbl = a.mutbl;
let (variance, info) = match mutbl {
ast::Mutability::Not => (ty::Covariant, ty::VarianceDiagInfo::None),
ast::Mutability::Mut => (ty::Invariant, ty::VarianceDiagInfo::Mut { kind, ty: a.ty }),
};
let ty = relation.relate_with_variance(variance, info, a.ty, b.ty)?;
Ok(ty::TypeAndMut { ty, mutbl })
}
}

Expand All @@ -142,7 +142,7 @@ pub fn relate_substs<R: TypeRelation<'tcx>>(

let params = iter::zip(a_subst, b_subst).enumerate().map(|(i, (a, b))| {
let variance = variances.map_or(ty::Invariant, |v| v[i]);
relation.relate_with_variance(variance, a, b)
relation.relate_with_variance(variance, ty::VarianceDiagInfo::default(), a, b)
});

tcx.mk_substs(params)
Expand Down Expand Up @@ -177,7 +177,12 @@ impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> {
if is_output {
relation.relate(a, b)
} else {
relation.relate_with_variance(ty::Contravariant, a, b)
relation.relate_with_variance(
ty::Contravariant,
ty::VarianceDiagInfo::default(),
a,
b,
)
}
})
.enumerate()
Expand Down Expand Up @@ -251,8 +256,18 @@ impl<'tcx> Relate<'tcx> for ty::ExistentialProjection<'tcx> {
b.item_def_id,
)))
} else {
let ty = relation.relate_with_variance(ty::Invariant, a.ty, b.ty)?;
let substs = relation.relate_with_variance(ty::Invariant, a.substs, b.substs)?;
let ty = relation.relate_with_variance(
ty::Invariant,
ty::VarianceDiagInfo::default(),
a.ty,
b.ty,
)?;
let substs = relation.relate_with_variance(
ty::Invariant,
ty::VarianceDiagInfo::default(),
a.substs,
b.substs,
)?;
Ok(ty::ExistentialProjection { item_def_id: a.item_def_id, substs, ty })
}
}
Expand Down Expand Up @@ -364,7 +379,12 @@ pub fn super_relate_tys<R: TypeRelation<'tcx>>(

(&ty::Dynamic(a_obj, a_region), &ty::Dynamic(b_obj, b_region)) => {
let region_bound = relation.with_cause(Cause::ExistentialRegionBound, |relation| {
relation.relate_with_variance(ty::Contravariant, a_region, b_region)
relation.relate_with_variance(
ty::Contravariant,
ty::VarianceDiagInfo::default(),
a_region,
b_region,
)
})?;
Ok(tcx.mk_dynamic(relation.relate(a_obj, b_obj)?, region_bound))
}
Expand Down Expand Up @@ -398,15 +418,20 @@ pub fn super_relate_tys<R: TypeRelation<'tcx>>(
}

(&ty::RawPtr(a_mt), &ty::RawPtr(b_mt)) => {
let mt = relation.relate(a_mt, b_mt)?;
let mt = relate_type_and_mut(relation, a_mt, b_mt, ty::VarianceDiagMutKind::RawPtr)?;
Ok(tcx.mk_ptr(mt))
}

(&ty::Ref(a_r, a_ty, a_mutbl), &ty::Ref(b_r, b_ty, b_mutbl)) => {
let r = relation.relate_with_variance(ty::Contravariant, a_r, b_r)?;
let r = relation.relate_with_variance(
ty::Contravariant,
ty::VarianceDiagInfo::default(),
a_r,
b_r,
)?;
let a_mt = ty::TypeAndMut { ty: a_ty, mutbl: a_mutbl };
let b_mt = ty::TypeAndMut { ty: b_ty, mutbl: b_mutbl };
let mt = relation.relate(a_mt, b_mt)?;
let mt = relate_type_and_mut(relation, a_mt, b_mt, ty::VarianceDiagMutKind::Ref)?;
Ok(tcx.mk_ref(r, mt))
}

Expand Down Expand Up @@ -536,8 +561,12 @@ pub fn super_relate_consts<R: TypeRelation<'tcx>>(
(ty::ConstKind::Unevaluated(au), ty::ConstKind::Unevaluated(bu))
if au.def == bu.def && au.promoted == bu.promoted =>
{
let substs =
relation.relate_with_variance(ty::Variance::Invariant, au.substs, bu.substs)?;
let substs = relation.relate_with_variance(
ty::Variance::Invariant,
ty::VarianceDiagInfo::default(),
au.substs,
bu.substs,
)?;
return Ok(tcx.mk_const(ty::Const {
val: ty::ConstKind::Unevaluated(ty::Unevaluated {
def: au.def,
Expand Down

0 comments on commit 35fff69

Please sign in to comment.