Auto merge of #89336 - Aaron1011:variance-struct-diag, r=cjgillot

Refactor variance diagnostics to work with more types Instead of special-casing mutable pointers/references, we now support general generic types (currently, we handle `ty::Ref`, `ty::RawPtr`, and `ty::Adt`) When a `ty::Adt` is involved, we show an additional note explaining which of the type's generic parameters is invariant (e.g. the `T` in `Cell<T>`). Currently, we don't explain *why* a particular generic parameter ends up becoming invariant. In the general case, this could require printing a long 'backtrace' of types, so doing this would be more suitable for a follow-up PR. We still only handle the case where our variance switches to `ty::Invariant`.
rust-lang · Dec 30, 2021 · f8d4ee7 · f8d4ee7
2 parents 65d8785 + b15cb29
commit f8d4ee7
Show file tree

Hide file tree

Showing 25 changed files with 247 additions and 40 deletions.
diff --git a/compiler/rustc_borrowck/src/diagnostics/region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/region_errors.rs
@@ -6,7 +6,7 @@ use rustc_infer::infer::{
     error_reporting::unexpected_hidden_region_diagnostic, NllRegionVariableOrigin,
 };
 use rustc_middle::mir::{ConstraintCategory, ReturnConstraint};
-use rustc_middle::ty::subst::Subst;
+use rustc_middle::ty::subst::{InternalSubsts, Subst};
 use rustc_middle::ty::{self, RegionVid, Ty};
 use rustc_span::symbol::{kw, sym};
 use rustc_span::{BytePos, Span};
@@ -334,13 +334,43 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
         match variance_info {
             ty::VarianceDiagInfo::None => {}
-            ty::VarianceDiagInfo::Mut { kind, ty } => {
-                let kind_name = match kind {
-                    ty::VarianceDiagMutKind::Ref => "reference",
-                    ty::VarianceDiagMutKind::RawPtr => "pointer",
+            ty::VarianceDiagInfo::Invariant { ty, param_index } => {
+                let (desc, note) = match ty.kind() {
+                    ty::RawPtr(ty_mut) => {
+                        assert_eq!(ty_mut.mutbl, rustc_hir::Mutability::Mut);
+                        (
+                            format!("a mutable pointer to {}", ty_mut.ty),
+                            "mutable pointers are invariant over their type parameter".to_string(),
+                        )
+                    }
+                    ty::Ref(_, inner_ty, mutbl) => {
+                        assert_eq!(*mutbl, rustc_hir::Mutability::Mut);
+                        (
+                            format!("a mutable reference to {}", inner_ty),
+                            "mutable references are invariant over their type parameter"
+                                .to_string(),
+                        )
+                    }
+                    ty::Adt(adt, substs) => {
+                        let generic_arg = substs[param_index as usize];
+                        let identity_substs =
+                            InternalSubsts::identity_for_item(self.infcx.tcx, adt.did);
+                        let base_ty = self.infcx.tcx.mk_adt(adt, identity_substs);
+                        let base_generic_arg = identity_substs[param_index as usize];
+                        let adt_desc = adt.descr();
+
+                        let desc = format!(
+                            "the type {ty}, which makes the generic argument {generic_arg} invariant"
+                        );
+                        let note = format!(
+                            "the {adt_desc} {base_ty} is invariant over the parameter {base_generic_arg}"
+                        );
+                        (desc, note)
+                    }
+                    _ => panic!("Unexpected type {:?}", ty),
                 };
-                diag.note(&format!("requirement occurs because of a mutable {kind_name} to {ty}",));
-                diag.note(&format!("mutable {kind_name}s are invariant over their type parameter"));
+                diag.note(&format!("requirement occurs because of {desc}",));
+                diag.note(&note);
                 diag.help("see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance");
             }
         }

diff --git a/compiler/rustc_infer/src/infer/combine.rs b/compiler/rustc_infer/src/infer/combine.rs
@@ -572,8 +572,9 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
             // (e.g., #41849).
             relate::relate_substs(self, None, a_subst, b_subst)
         } else {
-            let opt_variances = self.tcx().variances_of(item_def_id);
-            relate::relate_substs(self, Some(&opt_variances), a_subst, b_subst)
+            let tcx = self.tcx();
+            let opt_variances = tcx.variances_of(item_def_id);
+            relate::relate_substs(self, Some((item_def_id, &opt_variances)), a_subst, b_subst)
         }
     }
 

diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
@@ -77,7 +77,7 @@ pub use self::sty::{
     GeneratorSubsts, GeneratorSubstsParts, InlineConstSubsts, InlineConstSubstsParts, ParamConst,
     ParamTy, PolyExistentialProjection, PolyExistentialTraitRef, PolyFnSig, PolyGenSig,
     PolyTraitRef, ProjectionTy, Region, RegionKind, RegionVid, TraitRef, TyKind, TypeAndMut,
-    UpvarSubsts, VarianceDiagInfo, VarianceDiagMutKind,
+    UpvarSubsts, VarianceDiagInfo,
 };
 pub use self::trait_def::TraitDef;
 

diff --git a/compiler/rustc_middle/src/ty/relate.rs b/compiler/rustc_middle/src/ty/relate.rs
@@ -6,7 +6,7 @@
 
 use crate::mir::interpret::{get_slice_bytes, ConstValue, GlobalAlloc, Scalar};
 use crate::ty::error::{ExpectedFound, TypeError};
-use crate::ty::subst::{GenericArg, GenericArgKind, SubstsRef};
+use crate::ty::subst::{GenericArg, GenericArgKind, Subst, SubstsRef};
 use crate::ty::{self, Ty, TyCtxt, TypeFoldable};
 use rustc_hir as ast;
 use rustc_hir::def_id::DefId;
@@ -59,8 +59,9 @@ pub trait TypeRelation<'tcx>: Sized {
             item_def_id, a_subst, b_subst
         );
 
-        let opt_variances = self.tcx().variances_of(item_def_id);
-        relate_substs(self, Some(opt_variances), a_subst, b_subst)
+        let tcx = self.tcx();
+        let opt_variances = tcx.variances_of(item_def_id);
+        relate_substs(self, Some((item_def_id, opt_variances)), a_subst, b_subst)
     }
 
     /// Switch variance for the purpose of relating `a` and `b`.
@@ -116,7 +117,7 @@ pub fn relate_type_and_mut<'tcx, R: TypeRelation<'tcx>>(
     relation: &mut R,
     a: ty::TypeAndMut<'tcx>,
     b: ty::TypeAndMut<'tcx>,
-    kind: ty::VarianceDiagMutKind,
+    base_ty: Ty<'tcx>,
 ) -> RelateResult<'tcx, ty::TypeAndMut<'tcx>> {
     debug!("{}.mts({:?}, {:?})", relation.tag(), a, b);
     if a.mutbl != b.mutbl {
@@ -125,7 +126,9 @@ pub fn relate_type_and_mut<'tcx, R: TypeRelation<'tcx>>(
         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 }),
+            ast::Mutability::Mut => {
+                (ty::Invariant, ty::VarianceDiagInfo::Invariant { ty: base_ty, param_index: 0 })
+            }
         };
         let ty = relation.relate_with_variance(variance, info, a.ty, b.ty)?;
         Ok(ty::TypeAndMut { ty, mutbl })
@@ -134,15 +137,29 @@ pub fn relate_type_and_mut<'tcx, R: TypeRelation<'tcx>>(
 
 pub fn relate_substs<'tcx, R: TypeRelation<'tcx>>(
     relation: &mut R,
-    variances: Option<&[ty::Variance]>,
+    variances: Option<(DefId, &[ty::Variance])>,
     a_subst: SubstsRef<'tcx>,
     b_subst: SubstsRef<'tcx>,
 ) -> RelateResult<'tcx, SubstsRef<'tcx>> {
     let tcx = relation.tcx();
+    let mut cached_ty = None;
 
     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, ty::VarianceDiagInfo::default(), a, b)
+        let (variance, variance_info) = match variances {
+            Some((ty_def_id, variances)) => {
+                let variance = variances[i];
+                let variance_info = if variance == ty::Invariant {
+                    let ty =
+                        cached_ty.get_or_insert_with(|| tcx.type_of(ty_def_id).subst(tcx, a_subst));
+                    ty::VarianceDiagInfo::Invariant { ty, param_index: i.try_into().unwrap() }
+                } else {
+                    ty::VarianceDiagInfo::default()
+                };
+                (variance, variance_info)
+            }
+            None => (ty::Invariant, ty::VarianceDiagInfo::default()),
+        };
+        relation.relate_with_variance(variance, variance_info, a, b)
     });
 
     tcx.mk_substs(params)
@@ -436,7 +453,7 @@ pub fn super_relate_tys<'tcx, R: TypeRelation<'tcx>>(
         }
 
         (&ty::RawPtr(a_mt), &ty::RawPtr(b_mt)) => {
-            let mt = relate_type_and_mut(relation, a_mt, b_mt, ty::VarianceDiagMutKind::RawPtr)?;
+            let mt = relate_type_and_mut(relation, a_mt, b_mt, a)?;
             Ok(tcx.mk_ptr(mt))
         }
 
@@ -449,7 +466,7 @@ pub fn super_relate_tys<'tcx, R: TypeRelation<'tcx>>(
             )?;
             let a_mt = ty::TypeAndMut { ty: a_ty, mutbl: a_mutbl };
             let b_mt = ty::TypeAndMut { ty: b_ty, mutbl: b_mutbl };
-            let mt = relate_type_and_mut(relation, a_mt, b_mt, ty::VarianceDiagMutKind::Ref)?;
+            let mt = relate_type_and_mut(relation, a_mt, b_mt, a)?;
             Ok(tcx.mk_ref(r, mt))
         }
 

diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs
@@ -2282,36 +2282,26 @@ pub enum VarianceDiagInfo<'tcx> {
     /// We will not add any additional information to error messages.
     #[default]
     None,
-    /// We switched our variance because a type occurs inside
-    /// the generic argument of a mutable reference or pointer
-    /// (`*mut T` or `&mut T`). In either case, our variance
-    /// will always be `Invariant`.
-    Mut {
-        /// Tracks whether we had a mutable pointer or reference,
-        /// for better error messages
-        kind: VarianceDiagMutKind,
-        /// The type parameter of the mutable pointer/reference
-        /// (the `T` in `&mut T` or `*mut T`).
+    /// We switched our variance because a generic argument occurs inside
+    /// the invariant generic argument of another type.
+    Invariant {
+        /// The generic type containing the generic parameter
+        /// that changes the variance (e.g. `*mut T`, `MyStruct<T>`)
         ty: Ty<'tcx>,
+        /// The index of the generic parameter being used
+        /// (e.g. `0` for `*mut T`, `1` for `MyStruct<'CovariantParam, 'InvariantParam>`)
+        param_index: u32,
     },
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-pub enum VarianceDiagMutKind {
-    /// A mutable raw pointer (`*mut T`)
-    RawPtr,
-    /// A mutable reference (`&mut T`)
-    Ref,
-}
-
 impl<'tcx> VarianceDiagInfo<'tcx> {
     /// Mirrors `Variance::xform` - used to 'combine' the existing
     /// and new `VarianceDiagInfo`s when our variance changes.
     pub fn xform(self, other: VarianceDiagInfo<'tcx>) -> VarianceDiagInfo<'tcx> {
-        // For now, just use the first `VarianceDiagInfo::Mut` that we see
+        // For now, just use the first `VarianceDiagInfo::Invariant` that we see
         match self {
             VarianceDiagInfo::None => other,
-            VarianceDiagInfo::Mut { .. } => self,
+            VarianceDiagInfo::Invariant { .. } => self,
         }
     }
 }
diff --git a/src/test/ui/associated-types/cache/project-fn-ret-invariant.krisskross.nll.stderr b/src/test/ui/associated-types/cache/project-fn-ret-invariant.krisskross.nll.stderr
@@ -10,6 +10,9 @@ LL |     (a, b)
    |     ^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
    |
    = help: consider adding the following bound: `'a: 'b`
+   = note: requirement occurs because of the type Type<'_>, which makes the generic argument '_ invariant
+   = note: the struct Type<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/project-fn-ret-invariant.rs:56:5
@@ -23,6 +26,9 @@ LL |     (a, b)
    |     ^^^^^^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b`
    |
    = help: consider adding the following bound: `'b: 'a`
+   = note: requirement occurs because of the type Type<'_>, which makes the generic argument '_ invariant
+   = note: the struct Type<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 help: `'a` and `'b` must be the same: replace one with the other
 

diff --git a/src/test/ui/associated-types/cache/project-fn-ret-invariant.oneuse.nll.stderr b/src/test/ui/associated-types/cache/project-fn-ret-invariant.oneuse.nll.stderr
@@ -10,6 +10,9 @@ LL |     let a = bar(f, x);
    |             ^^^^^^^^^ argument requires that `'a` must outlive `'b`
    |
    = help: consider adding the following bound: `'a: 'b`
+   = note: requirement occurs because of the type Type<'_>, which makes the generic argument '_ invariant
+   = note: the struct Type<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/project-fn-ret-invariant.rs:40:13
@@ -23,6 +26,9 @@ LL |     let b = bar(f, y);
    |             ^^^^^^^^^ argument requires that `'b` must outlive `'a`
    |
    = help: consider adding the following bound: `'b: 'a`
+   = note: requirement occurs because of the type Type<'_>, which makes the generic argument '_ invariant
+   = note: the struct Type<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 help: `'a` and `'b` must be the same: replace one with the other
 

diff --git a/src/test/ui/associated-types/cache/project-fn-ret-invariant.transmute.nll.stderr b/src/test/ui/associated-types/cache/project-fn-ret-invariant.transmute.nll.stderr
@@ -6,6 +6,10 @@ LL | fn baz<'a, 'b>(x: Type<'a>) -> Type<'static> {
 ...
 LL |     bar(foo, x)
    |     ^^^^^^^^^^^ returning this value requires that `'a` must outlive `'static`
+   |
+   = note: requirement occurs because of the type Type<'_>, which makes the generic argument '_ invariant
+   = note: the struct Type<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/c-variadic/variadic-ffi-4.stderr b/src/test/ui/c-variadic/variadic-ffi-4.stderr
@@ -7,6 +7,10 @@ LL | pub unsafe extern "C" fn no_escape0<'f>(_: usize, ap: ...) -> VaListImpl<'f
    |                                     lifetime `'f` defined here
 LL |     ap
    |     ^^ function was supposed to return data with lifetime `'1` but it is returning data with lifetime `'f`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/variadic-ffi-4.rs:8:5
@@ -17,6 +21,10 @@ LL | pub unsafe extern "C" fn no_escape0<'f>(_: usize, ap: ...) -> VaListImpl<'f
    |                                     lifetime `'f` defined here
 LL |     ap
    |     ^^ returning this value requires that `'1` must outlive `'f`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/variadic-ffi-4.rs:14:5
@@ -25,6 +33,10 @@ LL | pub unsafe extern "C" fn no_escape1(_: usize, ap: ...) -> VaListImpl<'stati
    |                                               -- has type `VaListImpl<'1>`
 LL |     ap
    |     ^^ returning this value requires that `'1` must outlive `'static`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/variadic-ffi-4.rs:18:31
@@ -44,6 +56,10 @@ LL | pub unsafe extern "C" fn no_escape3(_: usize, mut ap0: &mut VaListImpl, mut
    |                                               has type `&mut VaListImpl<'1>`
 LL |     *ap0 = ap1;
    |     ^^^^ assignment requires that `'1` must outlive `'2`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/variadic-ffi-4.rs:22:5
@@ -54,6 +70,10 @@ LL | pub unsafe extern "C" fn no_escape3(_: usize, mut ap0: &mut VaListImpl, mut
    |                                               has type `&mut VaListImpl<'1>`
 LL |     *ap0 = ap1;
    |     ^^^^ assignment requires that `'2` must outlive `'1`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/variadic-ffi-4.rs:28:5
@@ -106,6 +126,10 @@ LL | pub unsafe extern "C" fn no_escape5(_: usize, mut ap0: &mut VaListImpl, mut
    |                                               has type `&mut VaListImpl<'1>`
 LL |     *ap0 = ap1.clone();
    |            ^^^^^^^^^^^ argument requires that `'1` must outlive `'2`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: lifetime may not live long enough
   --> $DIR/variadic-ffi-4.rs:35:12
@@ -116,6 +140,10 @@ LL | pub unsafe extern "C" fn no_escape5(_: usize, mut ap0: &mut VaListImpl, mut
    |                                               has type `&mut VaListImpl<'1>`
 LL |     *ap0 = ap1.clone();
    |            ^^^^^^^^^^^ argument requires that `'2` must outlive `'1`
+   |
+   = note: requirement occurs because of the type VaListImpl<'_>, which makes the generic argument '_ invariant
+   = note: the struct VaListImpl<'f> is invariant over the parameter 'f
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: aborting due to 11 previous errors
 

diff --git a/src/test/ui/hr-subtype/hr-subtype.free_inv_x_vs_free_inv_y.nll.stderr b/src/test/ui/hr-subtype/hr-subtype.free_inv_x_vs_free_inv_y.nll.stderr
@@ -13,6 +13,9 @@ LL | | fn(Inv<'y>)) }
    | |______________- in this macro invocation
    |
    = help: consider adding the following bound: `'x: 'y`
+   = note: requirement occurs because of the type Inv<'_>, which makes the generic argument '_ invariant
+   = note: the struct Inv<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
    = note: this error originates in the macro `check` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: lifetime may not live long enough
@@ -30,6 +33,9 @@ LL | | fn(Inv<'y>)) }
    | |______________- in this macro invocation
    |
    = help: consider adding the following bound: `'x: 'y`
+   = note: requirement occurs because of the type Inv<'_>, which makes the generic argument '_ invariant
+   = note: the struct Inv<'a> is invariant over the parameter 'a
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
    = note: this error originates in the macro `check` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: aborting due to 2 previous errors

diff --git a/...test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr b/...test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr
@@ -51,6 +51,10 @@ LL | |     });
    | |      |
    | |______`cell_a` escapes the function body here
    |        argument requires that `'a` must outlive `'static`
+   |
+   = note: requirement occurs because of the type Cell<&'_#10r u32>, which makes the generic argument &'_#10r u32 invariant
+   = note: the struct Cell<T> is invariant over the parameter T
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: aborting due to previous error
 

diff --git a/...t/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr b/...t/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr
@@ -51,6 +51,10 @@ LL | |     });
    | |      |
    | |______`cell_a` escapes the function body here
    |        argument requires that `'a` must outlive `'static`
+   |
+   = note: requirement occurs because of the type Cell<&'_#11r u32>, which makes the generic argument &'_#11r u32 invariant
+   = note: the struct Cell<T> is invariant over the parameter T
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: aborting due to previous error
 

diff --git a/src/test/ui/nll/where_clauses_in_structs.stderr b/src/test/ui/nll/where_clauses_in_structs.stderr
@@ -9,6 +9,9 @@ LL |     Foo { x, y };
    |           ^ this usage requires that `'a` must outlive `'b`
    |
    = help: consider adding the following bound: `'a: 'b`
+   = note: requirement occurs because of the type Cell<&u32>, which makes the generic argument &u32 invariant
+   = note: the struct Cell<T> is invariant over the parameter T
+   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: aborting due to previous error