Prevent opaque types being instantiated twice with different regions within the same function

Author: oli-obk

compiler/rustc_borrowck/messages.ftl

@@ -129,6 +129,12 @@ borrowck_moved_due_to_usage_in_operator =
         *[false] operator
     }
 
+borrowck_opaque_type_lifetime_mismatch =
+    opaque type used twice with different lifetimes
+    .label = lifetime `{$arg}` used here
+    .prev_lifetime_label = lifetime `{$prev}` previously used here
+    .note = if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types.
+
 borrowck_opaque_type_non_generic_param =
     expected generic {$kind} parameter, found `{$ty}`
     .label = {STREQ($ty, "'static") ->

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -13,11 +13,68 @@ use rustc_span::Span;
 use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
 use rustc_trait_selection::traits::ObligationCtxt;
 
+use crate::session_diagnostics::LifetimeMismatchOpaqueParam;
 use crate::session_diagnostics::NonGenericOpaqueTypeParam;
 
 use super::RegionInferenceContext;
 
 impl<'tcx> RegionInferenceContext<'tcx> {
+    fn universal_name(&self, vid: ty::RegionVid) -> Option<ty::Region<'tcx>> {
+        let scc = self.constraint_sccs.scc(vid);
+        self.scc_values
+            .universal_regions_outlived_by(scc)
+            .find_map(|lb| self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?))
+    }
+
+    fn gen_arg_to_name(&self, arg: ty::GenericArg<'tcx>) -> Option<ty::Region<'tcx>> {
+        let region = arg.as_region()?;
+
+        if let ty::RePlaceholder(..) = region.kind() {
+            return None;
+        }
+        self.universal_name(self.to_region_vid(region))
+    }
+
+    /// Check that all opaque types have the same region parameters if they have the same
+    /// non-region parameters. This is necessary because within the new solver we perform various query operations
+    /// modulo regions, and thus could unsoundly select some impls that don't hold.
+    fn check_unique(
+        &self,
+        infcx: &InferCtxt<'tcx>,
+        opaque_ty_decls: &FxIndexMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
+    ) {
+        for (i, (a, a_ty)) in opaque_ty_decls.iter().enumerate() {
+            for (b, b_ty) in opaque_ty_decls.iter().skip(i + 1) {
+                if a.def_id != b.def_id {
+                    continue;
+                }
+                // Non-lifetime params differ -> ok
+                if infcx.tcx.erase_regions(a.args) != infcx.tcx.erase_regions(b.args) {
+                    continue;
+                }
+                trace!(?a, ?b);
+                for (a, b) in a.args.iter().zip(b.args) {
+                    if a.as_region().is_none() {
+                        break;
+                    }
+                    let a = self.gen_arg_to_name(a).unwrap();
+                    let b = self.gen_arg_to_name(b).unwrap();
+                    trace!(?a, ?b);
+                    if a == b {
+                        continue;
+                    }
+
+                    infcx.tcx.sess.emit_err(LifetimeMismatchOpaqueParam {
+                        arg: a.into(),
+                        prev: b.into(),
+                        span: a_ty.span,
+                        prev_span: b_ty.span,
+                    });
+                }
+            }
+        }
+    }
+
     /// Resolve any opaque types that were encountered while borrow checking
     /// this item. This is then used to get the type in the `type_of` query.
     ///
@@ -63,6 +120,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         infcx: &InferCtxt<'tcx>,
         opaque_ty_decls: FxIndexMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>>,
     ) -> FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>> {
+        self.check_unique(infcx, &opaque_ty_decls);
+
         let mut result: FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>> = FxIndexMap::default();
 
         let member_constraints: FxIndexMap<_, _> = self
@@ -78,13 +137,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
             let mut subst_regions = vec![self.universal_regions.fr_static];
 
-            let to_universal_region = |vid, subst_regions: &mut Vec<_>| {
-                trace!(?vid);
-                let scc = self.constraint_sccs.scc(vid);
-                trace!(?scc);
-                match self.scc_values.universal_regions_outlived_by(scc).find_map(|lb| {
-                    self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?)
-                }) {
+            let to_universal_region =
+                |vid, subst_regions: &mut Vec<_>| match self.universal_name(vid) {
                     Some(region) => {
                         let vid = self.universal_regions.to_region_vid(region);
                         subst_regions.push(vid);
@@ -98,8 +152,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                             "opaque type with non-universal region args",
                         )
                     }
-                }
-            };
+                };
 
             // Start by inserting universal regions from the member_constraint choice regions.
             // This will ensure they get precedence when folding the regions in the concrete type.

compiler/rustc_borrowck/src/session_diagnostics.rs

@@ -304,6 +304,19 @@ pub(crate) struct NonGenericOpaqueTypeParam<'a, 'tcx> {
     pub param_span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(borrowck_opaque_type_lifetime_mismatch)]
+pub(crate) struct LifetimeMismatchOpaqueParam<'tcx> {
+    pub arg: GenericArg<'tcx>,
+    pub prev: GenericArg<'tcx>,
+    #[primary_span]
+    #[label]
+    #[note]
+    pub span: Span,
+    #[label(borrowck_prev_lifetime_label)]
+    pub prev_span: Span,
+}
+
 #[derive(Subdiagnostic)]
 pub(crate) enum CaptureReasonLabel<'a> {
     #[label(borrowck_moved_due_to_call)]

tests/ui/impl-trait/issue-86465.rs

@@ -0,0 +1,22 @@
+#![feature(type_alias_impl_trait)]
+
+type Foo<'a> = impl Sized;
+
+fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> (Foo<'a>, Foo<'b>) {
+    (x, y)
+    //~^ ERROR opaque type used twice with different lifetimes
+}
+
+type Bar<'a, 'b> = impl std::fmt::Debug;
+
+fn bar<'x, 'y>(i: &'x i32, j: &'y i32) -> (Bar<'x, 'y>, Bar<'y, 'x>) {
+    (i, j)
+    //~^ ERROR opaque type used twice with different lifetimes
+    //~| ERROR opaque type used twice with different lifetimes
+}
+
+fn main() {
+    let meh = 42;
+    let muh = 69;
+    println!("{:?}", bar(&meh, &muh));
+}

tests/ui/impl-trait/issue-86465.stderr

@@ -0,0 +1,47 @@
+error: opaque type used twice with different lifetimes
+  --> $DIR/lifetime_mismatch.rs:6:5
+   |
+LL |     (x, y)
+   |     ^^^^^^
+   |     |
+   |     lifetime `'a` used here
+   |     lifetime `'b` previously used here
+   |
+note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types.
+  --> $DIR/lifetime_mismatch.rs:6:5
+   |
+LL |     (x, y)
+   |     ^^^^^^
+
+error: opaque type used twice with different lifetimes
+  --> $DIR/lifetime_mismatch.rs:13:5
+   |
+LL |     (i, j)
+   |     ^^^^^^
+   |     |
+   |     lifetime `'x` used here
+   |     lifetime `'y` previously used here
+   |
+note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types.
+  --> $DIR/lifetime_mismatch.rs:13:5
+   |
+LL |     (i, j)
+   |     ^^^^^^
+
+error: opaque type used twice with different lifetimes
+  --> $DIR/lifetime_mismatch.rs:13:5
+   |
+LL |     (i, j)
+   |     ^^^^^^
+   |     |
+   |     lifetime `'y` used here
+   |     lifetime `'x` previously used here
+   |
+note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types.
+  --> $DIR/lifetime_mismatch.rs:13:5
+   |
+LL |     (i, j)
+   |     ^^^^^^
+
+error: aborting due to 3 previous errors
+

tests/ui/type-alias-impl-trait/lifetime_mismatch.rs

@@ -3,7 +3,9 @@
 type Foo<'a, 'b> = impl std::fmt::Debug;
 
 fn foo<'x, 'y>(i: &'x i32, j: &'y i32) -> (Foo<'x, 'y>, Foo<'y, 'x>) {
-    (i, i) //~ ERROR concrete type differs from previous
+    (i, i)
+    //~^ ERROR opaque type used twice with different lifetimes
+    //~| ERROR opaque type used twice with different lifetimes
 }
 
 fn main() {}

tests/ui/type-alias-impl-trait/lifetime_mismatch.stderr

@@ -1,11 +1,32 @@
-error: concrete type differs from previous defining opaque type use
+error: opaque type used twice with different lifetimes
   --> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
    |
 LL |     (i, i)
    |     ^^^^^^
    |     |
-   |     expected `&'a i32`, got `&'b i32`
-   |     this expression supplies two conflicting concrete types for the same opaque type
+   |     lifetime `'x` used here
+   |     lifetime `'y` previously used here
+   |
+note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types.
+  --> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
+   |
+LL |     (i, i)
+   |     ^^^^^^
+
+error: opaque type used twice with different lifetimes
+  --> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
+   |
+LL |     (i, i)
+   |     ^^^^^^
+   |     |
+   |     lifetime `'y` used here
+   |     lifetime `'x` previously used here
+   |
+note: if all non-lifetime generic parameters are the same, but the lifetime parameters differ, it is not possible to differentiate the opaque types.
+  --> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5
+   |
+LL |     (i, i)
+   |     ^^^^^^
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 

tests/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs

@@ -7,15 +7,6 @@ fn f<A: ToString + Clone, B: ToString + Clone>(a: A, b: B) -> (X<A, B>, X<A, B>)
     (a.clone(), a)
 }
 
-type Foo<'a, 'b> = impl std::fmt::Debug;
-
-fn foo<'x, 'y>(i: &'x i32, j: &'y i32) -> (Foo<'x, 'y>, Foo<'y, 'x>) {
-    (i, j)
-}
-
 fn main() {
     println!("{}", <X<_, _> as ToString>::to_string(&f(42_i32, String::new()).1));
-    let meh = 42;
-    let muh = 69;
-    println!("{:?}", foo(&meh, &muh));
 }