Auto merge of rust-lang#118247 - spastorino:type-equality-subtyping, …

…r=<try>

Fix for TypeId exposes equality-by-subtyping vs normal-form-syntactic-equality unsoundness

Fixes rust-lang#97156

This PR revives rust-lang#97427 idea.

r? `@lcnr`

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -138,6 +138,7 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
     let name_str = intrinsic_name.as_str();
 
     let bound_vars = tcx.mk_bound_variable_kinds(&[
+        ty::BoundVariableKind::Region(ty::BrAnon),
         ty::BoundVariableKind::Region(ty::BrAnon),
         ty::BoundVariableKind::Region(ty::BrEnv),
     ]);
@@ -151,7 +152,7 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
             let env_region = ty::Region::new_bound(
                 tcx,
                 ty::INNERMOST,
-                ty::BoundRegion { var: ty::BoundVar::from_u32(1), kind: ty::BrEnv },
+                ty::BoundRegion { var: ty::BoundVar::from_u32(2), kind: ty::BrEnv },
             );
             let va_list_ty = tcx.type_of(did).instantiate(tcx, &[region.into()]);
             (Ty::new_ref(tcx, env_region, ty::TypeAndMut { ty: va_list_ty, mutbl }), va_list_ty)
@@ -446,9 +447,12 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
 
             sym::raw_eq => {
                 let br = ty::BoundRegion { var: ty::BoundVar::from_u32(0), kind: ty::BrAnon };
-                let param_ty =
+                let param_ty_lhs =
+                    Ty::new_imm_ref(tcx, ty::Region::new_bound(tcx, ty::INNERMOST, br), param(0));
+                let br = ty::BoundRegion { var: ty::BoundVar::from_u32(1), kind: ty::BrAnon };
+                let param_ty_rhs =
                     Ty::new_imm_ref(tcx, ty::Region::new_bound(tcx, ty::INNERMOST, br), param(0));
-                (1, vec![param_ty; 2], tcx.types.bool)
+                (1, vec![param_ty_lhs, param_ty_rhs], tcx.types.bool)
             }
 
             sym::black_box => (1, vec![param(0)], param(0)),

compiler/rustc_infer/src/infer/equate.rs

@@ -165,8 +165,8 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
         }
 
         if a.skip_binder().has_escaping_bound_vars() || b.skip_binder().has_escaping_bound_vars() {
-            self.fields.higher_ranked_sub(a, b, self.a_is_expected)?;
-            self.fields.higher_ranked_sub(b, a, self.a_is_expected)?;
+            self.fields.higher_ranked_equate(a, b, self.a_is_expected)?;
+            self.fields.higher_ranked_equate(b, a, !self.a_is_expected)?;
         } else {
             // Fast path for the common case.
             self.relate(a.skip_binder(), b.skip_binder())?;

compiler/rustc_infer/src/infer/higher_ranked/mod.rs

@@ -57,6 +57,40 @@ impl<'a, 'tcx> CombineFields<'a, 'tcx> {
         // placeholders which **must not** be named afterwards.
         Ok(())
     }
+
+    #[instrument(skip(self), level = "debug")]
+    pub fn higher_ranked_equate<T>(
+        &mut self,
+        a: Binder<'tcx, T>,
+        b: Binder<'tcx, T>,
+        a_is_expected: bool,
+    ) -> RelateResult<'tcx, ()>
+    where
+        T: Relate<'tcx>,
+    {
+        let span = self.trace.cause.span;
+        // First, we instantiate each bound region in the supertype with a
+        // fresh placeholder region. Note that this automatically creates
+        // a new universe if needed.
+        let b_prime = self.infcx.instantiate_binder_with_placeholders(b);
+
+        // Next, we instantiate each bound region in the subtype
+        // with a fresh region variable. These region variables --
+        // but no other preexisting region variables -- can name
+        // the placeholders.
+        let a_prime = self.infcx.instantiate_binder_with_fresh_vars(span, HigherRankedType, a);
+
+        debug!("a_prime={:?}", a_prime);
+        debug!("b_prime={:?}", b_prime);
+
+        // Compare types now that bound regions have been replaced.
+        let result = self.equate(a_is_expected).relate(a_prime, b_prime)?;
+
+        debug!("OK result={result:?}");
+        // NOTE: returning the result here would be dangerous as it contains
+        // placeholders which **must not** be named afterwards.
+        Ok(())
+    }
 }
 
 impl<'tcx> InferCtxt<'tcx> {

compiler/rustc_infer/src/infer/nll_relate/mod.rs

@@ -595,69 +595,84 @@ where
         // - Instantiate binders on `b` universally, yielding a universe U1.
         // - Instantiate binders on `a` existentially in U1.
 
-        debug!(?self.ambient_variance);
-
         if let (Some(a), Some(b)) = (a.no_bound_vars(), b.no_bound_vars()) {
             // Fast path for the common case.
             self.relate(a, b)?;
             return Ok(ty::Binder::dummy(a));
         }
 
-        if self.ambient_covariance() {
-            // Covariance, so we want `for<..> A <: for<..> B` --
-            // therefore we compare any instantiation of A (i.e., A
-            // instantiated with existentials) against every
-            // instantiation of B (i.e., B instantiated with
-            // universals).
-
-            // Reset the ambient variance to covariant. This is needed
-            // to correctly handle cases like
-            //
-            //     for<'a> fn(&'a u32, &'a u32) == for<'b, 'c> fn(&'b u32, &'c u32)
-            //
-            // Somewhat surprisingly, these two types are actually
-            // **equal**, even though the one on the right looks more
-            // polymorphic. The reason is due to subtyping. To see it,
-            // consider that each function can call the other:
-            //
-            // - The left function can call the right with `'b` and
-            //   `'c` both equal to `'a`
-            //
-            // - The right function can call the left with `'a` set to
-            //   `{P}`, where P is the point in the CFG where the call
-            //   itself occurs. Note that `'b` and `'c` must both
-            //   include P. At the point, the call works because of
-            //   subtyping (i.e., `&'b u32 <: &{P} u32`).
-            let variance = std::mem::replace(&mut self.ambient_variance, ty::Variance::Covariant);
-
-            // Note: the order here is important. Create the placeholders first, otherwise
-            // we assign the wrong universe to the existential!
-            let b_replaced = self.instantiate_binder_with_placeholders(b);
-            let a_replaced = self.instantiate_binder_with_existentials(a);
-
-            self.relate(a_replaced, b_replaced)?;
-
-            self.ambient_variance = variance;
-        }
+        match self.ambient_variance {
+            ty::Variance::Covariant => {
+                // Covariance, so we want `for<..> A <: for<..> B` --
+                // therefore we compare any instantiation of A (i.e., A
+                // instantiated with existentials) against every
+                // instantiation of B (i.e., B instantiated with
+                // universals).
+
+                // Reset the ambient variance to covariant. This is needed
+                // to correctly handle cases like
+                //
+                //     for<'a> fn(&'a u32, &'a u32) == for<'b, 'c> fn(&'b u32, &'c u32)
+                //
+                // Somewhat surprisingly, these two types are actually
+                // **equal**, even though the one on the right looks more
+                // polymorphic. The reason is due to subtyping. To see it,
+                // consider that each function can call the other:
+                //
+                // - The left function can call the right with `'b` and
+                //   `'c` both equal to `'a`
+                //
+                // - The right function can call the left with `'a` set to
+                //   `{P}`, where P is the point in the CFG where the call
+                //   itself occurs. Note that `'b` and `'c` must both
+                //   include P. At the point, the call works because of
+                //   subtyping (i.e., `&'b u32 <: &{P} u32`).
+                let variance =
+                    std::mem::replace(&mut self.ambient_variance, ty::Variance::Covariant);
+
+                // Note: the order here is important. Create the placeholders first, otherwise
+                // we assign the wrong universe to the existential!
+                let b_replaced = self.instantiate_binder_with_placeholders(b);
+                let a_replaced = self.instantiate_binder_with_existentials(a);
+
+                self.relate(a_replaced, b_replaced)?;
+
+                self.ambient_variance = variance;
+            }
 
-        if self.ambient_contravariance() {
-            // Contravariance, so we want `for<..> A :> for<..> B`
-            // -- therefore we compare every instantiation of A (i.e.,
-            // A instantiated with universals) against any
-            // instantiation of B (i.e., B instantiated with
-            // existentials). Opposite of above.
+            ty::Variance::Contravariant => {
+                // Contravariance, so we want `for<..> A :> for<..> B`
+                // -- therefore we compare every instantiation of A (i.e.,
+                // A instantiated with universals) against any
+                // instantiation of B (i.e., B instantiated with
+                // existentials). Opposite of above.
+
+                // Reset ambient variance to contravariance. See the
+                // covariant case above for an explanation.
+                let variance =
+                    std::mem::replace(&mut self.ambient_variance, ty::Variance::Contravariant);
+
+                let a_replaced = self.instantiate_binder_with_placeholders(a);
+                let b_replaced = self.instantiate_binder_with_existentials(b);
 
-            // Reset ambient variance to contravariance. See the
-            // covariant case above for an explanation.
-            let variance =
-                std::mem::replace(&mut self.ambient_variance, ty::Variance::Contravariant);
+                self.relate(a_replaced, b_replaced)?;
 
-            let a_replaced = self.instantiate_binder_with_placeholders(a);
-            let b_replaced = self.instantiate_binder_with_existentials(b);
+                self.ambient_variance = variance;
+            }
+
+            ty::Variance::Invariant => {
+                // Note: the order here is important. Create the placeholders first, otherwise
+                // we assign the wrong universe to the existential!
+                let b_replaced = self.instantiate_binder_with_placeholders(b);
+                let a_replaced = self.instantiate_binder_with_existentials(a);
+                self.relate(a_replaced, b_replaced)?;
 
-            self.relate(a_replaced, b_replaced)?;
+                let a_replaced = self.instantiate_binder_with_placeholders(a);
+                let b_replaced = self.instantiate_binder_with_existentials(b);
+                self.relate(a_replaced, b_replaced)?;
+            }
 
-            self.ambient_variance = variance;
+            ty::Variance::Bivariant => {}
         }
 
         Ok(a)

tests/ui/associated-inherent-types/issue-111404-1.rs

@@ -8,7 +8,7 @@ impl<'a> Foo<fn(&'a ())> {
 }
 
 fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
-//~^ ERROR higher-ranked subtype error
-//~| ERROR higher-ranked subtype error
+//~^ ERROR mismatched types [E0308]
+//~| ERROR mismatched types [E0308]
 
 fn main() {}

tests/ui/associated-inherent-types/issue-111404-1.stderr

@@ -1,16 +1,22 @@
-error: higher-ranked subtype error
-  --> $DIR/issue-111404-1.rs:10:1
+error[E0308]: mismatched types
+  --> $DIR/issue-111404-1.rs:10:11
    |
 LL | fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
+   |
+   = note: expected struct `Foo<fn(&())>`
+              found struct `Foo<for<'b> fn(&'b ())>`
 
-error: higher-ranked subtype error
-  --> $DIR/issue-111404-1.rs:10:1
+error[E0308]: mismatched types
+  --> $DIR/issue-111404-1.rs:10:11
    |
 LL | fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
    |
+   = note: expected struct `Foo<fn(&())>`
+              found struct `Foo<for<'b> fn(&'b ())>`
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
 error: aborting due to 2 previous errors
 
+For more information about this error, try `rustc --explain E0308`.

tests/ui/closure-expected-type/expect-fn-supply-fn.rs

@@ -30,14 +30,16 @@ fn expect_free_supply_bound() {
     // Here, we are given a function whose region is bound at closure level,
     // but we expect one bound in the argument. Error results.
     with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
-    //~^ ERROR mismatched types
+    //~^ ERROR mismatched types [E0308]
+    //~| ERROR lifetime may not live long enough
 }
 
 fn expect_bound_supply_free_from_fn<'x>(x: &'x u32) {
     // Here, we are given a `fn(&u32)` but we expect a `fn(&'x
     // u32)`. In principle, this could be ok, but we demand equality.
     with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
-    //~^ ERROR mismatched types
+    //~^ ERROR mismatched types [E0308]
+    //~| ERROR lifetime may not live long enough
 }
 
 fn expect_bound_supply_free_from_closure() {

tests/ui/closure-expected-type/expect-fn-supply-fn.stderr

@@ -19,6 +19,15 @@ LL | fn expect_free_supply_free_from_fn<'x>(x: &'x u32) {
 LL |     with_closure_expecting_fn_with_free_region(|x: fn(&'x u32), y| {});
    |                                                 ^ requires that `'x` must outlive `'static`
 
+error: lifetime may not live long enough
+  --> $DIR/expect-fn-supply-fn.rs:32:49
+   |
+LL |     with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
+   |                                                 ^
+   |                                                 |
+   |                                                 has type `fn(&'1 u32)`
+   |                                                 requires that `'1` must outlive `'static`
+
 error[E0308]: mismatched types
   --> $DIR/expect-fn-supply-fn.rs:32:49
    |
@@ -29,23 +38,32 @@ LL |     with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
               found fn pointer `for<'a> fn(&'a u32)`
 
 error[E0308]: mismatched types
-  --> $DIR/expect-fn-supply-fn.rs:39:50
+  --> $DIR/expect-fn-supply-fn.rs:40:50
    |
 LL |     with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
    |                                                  ^ one type is more general than the other
    |
    = note: expected fn pointer `for<'a> fn(&'a u32)`
               found fn pointer `fn(&u32)`
 
+error: lifetime may not live long enough
+  --> $DIR/expect-fn-supply-fn.rs:40:50
+   |
+LL | fn expect_bound_supply_free_from_fn<'x>(x: &'x u32) {
+   |                                     -- lifetime `'x` defined here
+...
+LL |     with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
+   |                                                  ^ requires that `'x` must outlive `'static`
+
 error[E0308]: mismatched types
-  --> $DIR/expect-fn-supply-fn.rs:48:50
+  --> $DIR/expect-fn-supply-fn.rs:50:50
    |
 LL |     with_closure_expecting_fn_with_bound_region(|x: Foo<'_>, y| {
    |                                                  ^ one type is more general than the other
    |
    = note: expected fn pointer `for<'a> fn(&'a u32)`
               found fn pointer `fn(&u32)`
 
-error: aborting due to 5 previous errors
+error: aborting due to 7 previous errors
 
 For more information about this error, try `rustc --explain E0308`.

tests/ui/coherence/coherence-fn-covariant-bound-vs-static.rs

@@ -1,3 +1,5 @@
+// check-pass
+
 // Test that impls for these two types are considered ovelapping:
 //
 // * `for<'r> fn(fn(&'r u32))`
@@ -15,7 +17,8 @@ trait Trait {}
 
 impl Trait for for<'r> fn(fn(&'r ())) {}
 impl<'a> Trait for fn(fn(&'a ())) {}
-//~^ ERROR conflicting implementations
+//~^ WARN conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))` [coherence_leak_check]
+//~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 //
 // Note in particular that we do NOT get a future-compatibility warning
 // here. This is because the new leak-check proposed in [MCP 295] does not

tests/ui/coherence/coherence-fn-covariant-bound-vs-static.stderr

@@ -1,13 +1,15 @@
-error[E0119]: conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))`
-  --> $DIR/coherence-fn-covariant-bound-vs-static.rs:17:1
+warning: conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))`
+  --> $DIR/coherence-fn-covariant-bound-vs-static.rs:19:1
    |
 LL | impl Trait for for<'r> fn(fn(&'r ())) {}
    | ------------------------------------- first implementation here
 LL | impl<'a> Trait for fn(fn(&'a ())) {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'r> fn(fn(&'r ()))`
    |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #56105 <https://github.com/rust-lang/rust/issues/56105>
    = note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details
+   = note: `#[warn(coherence_leak_check)]` on by default
 
-error: aborting due to 1 previous error
+warning: 1 warning emitted
 
-For more information about this error, try `rustc --explain E0119`.

tests/ui/coherence/coherence-fn-inputs.rs

@@ -1,3 +1,5 @@
+// check-pass
+
 // Test that we consider these two types completely equal:
 //
 // * `for<'a, 'b> fn(&'a u32, &'b u32)`
@@ -13,7 +15,8 @@
 trait Trait {}
 impl Trait for for<'a, 'b> fn(&'a u32, &'b u32) {}
 impl Trait for for<'c> fn(&'c u32, &'c u32) {
-    //~^ ERROR conflicting implementations
+    //~^ WARN conflicting implementations of trait `Trait` for type `for<'a, 'b> fn(&'a u32, &'b u32)` [coherence_leak_check]
+    //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
     //
     // Note in particular that we do NOT get a future-compatibility warning
     // here. This is because the new leak-check proposed in [MCP 295] does not