Skip to content

change equate for binders to not rely on subtyping #118247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 52 additions & 54 deletions compiler/rustc_borrowck/src/type_check/relate_tys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,61 +483,59 @@ impl<'bccx, 'tcx> TypeRelation<'tcx> for NllTypeRelating<'_, 'bccx, 'tcx> {
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!
self.enter_forall(b, |this, b| {
let a = this.instantiate_binder_with_existentials(a);
this.relate(a, b)
})?;

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).

// Note: the order here is important. Create the placeholders first, otherwise
// we assign the wrong universe to the existential!
self.enter_forall(b, |this, b| {
let a = this.instantiate_binder_with_existentials(a);
this.relate(a, b)
})?;
}

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.

// 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.enter_forall(a, |this, a| {
let b = this.instantiate_binder_with_existentials(b);
this.relate(a, b)
})?;

self.ambient_variance = variance;
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.

// Note: the order here is important. Create the placeholders first, otherwise
// we assign the wrong universe to the existential!
self.enter_forall(a, |this, a| {
let b = this.instantiate_binder_with_existentials(b);
this.relate(a, b)
})?;
}

ty::Variance::Invariant => {
// Invariant, so we want `for<..> A == for<..> B` --
// therefore we want `exists<..> A == for<..> B` and
// `exists<..> B == for<..> A`.
//
// See the comment in `fn Equate::binders` for more details.

// Note: the order here is important. Create the placeholders first, otherwise
// we assign the wrong universe to the existential!
self.enter_forall(b, |this, b| {
let a = this.instantiate_binder_with_existentials(a);
this.relate(a, b)
})?;
// Note: the order here is important. Create the placeholders first, otherwise
// we assign the wrong universe to the existential!
self.enter_forall(a, |this, a| {
let b = this.instantiate_binder_with_existentials(b);
this.relate(a, b)
})?;
}

ty::Variance::Bivariant => {}
}

Ok(a)
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub fn check_intrinsic_type(
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),
]);
Expand All @@ -181,7 +182,7 @@ pub fn check_intrinsic_type(
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)
Expand Down Expand Up @@ -493,9 +494,12 @@ pub fn check_intrinsic_type(

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, 0, vec![param_ty; 2], tcx.types.bool)
(1, 0, vec![param_ty_lhs, param_ty_rhs], tcx.types.bool)
}

sym::black_box => (1, 0, vec![param(0)], param(0)),
Expand Down
35 changes: 29 additions & 6 deletions compiler/rustc_infer/src/infer/relate/equate.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use super::combine::{CombineFields, ObligationEmittingRelation};
use super::StructurallyRelateAliases;
use crate::infer::BoundRegionConversionTime::HigherRankedType;
use crate::infer::{DefineOpaqueTypes, SubregionOrigin};
use crate::traits::PredicateObligations;

use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation};
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::TyVar;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{self, Ty, TyCtxt};

use rustc_hir::def_id::DefId;
use rustc_span::Span;
Expand Down Expand Up @@ -167,12 +168,34 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
return Ok(a);
}

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)?;
} else {
if let (Some(a), Some(b)) = (a.no_bound_vars(), b.no_bound_vars()) {
// Fast path for the common case.
self.relate(a.skip_binder(), b.skip_binder())?;
self.relate(a, b)?;
} else {
// When equating binders, we check that there is a 1-to-1
// correspondence between the bound vars in both types.
//
// We do so by separately instantiating one of the binders with
// placeholders and the other with inference variables and then
// equating the instantiated types.
//
// We want `for<..> A == for<..> B` -- therefore we want
// `exists<..> A == for<..> B` and `exists<..> B == for<..> A`.

let span = self.fields.trace.cause.span;
let infcx = self.fields.infcx;

// Check if `exists<..> A == for<..> B`
infcx.enter_forall(b, |b| {
let a = infcx.instantiate_binder_with_fresh_vars(span, HigherRankedType, a);
self.relate(a, b)
})?;

// Check if `exists<..> B == for<..> A`.
infcx.enter_forall(a, |a| {
let b = infcx.instantiate_binder_with_fresh_vars(span, HigherRankedType, b);
self.relate(a, b)
})?;
}
Ok(a)
}
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/associated-inherent-types/issue-111404-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ impl<'a> Foo<fn(&'a ())> {
}

fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
//~^ ERROR higher-ranked subtype error
//~^ ERROR mismatched types [E0308]
//~| ERROR mismatched types [E0308]
//~| ERROR higher-ranked subtype error
//~| ERROR higher-ranked subtype error

fn main() {}
22 changes: 21 additions & 1 deletion tests/ui/associated-inherent-types/issue-111404-1.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,22 @@
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[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: higher-ranked subtype error
--> $DIR/issue-111404-1.rs:10:1
|
Expand All @@ -12,5 +31,6 @@ LL | fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0308`.
6 changes: 4 additions & 2 deletions tests/ui/closure-expected-type/expect-fn-supply-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
24 changes: 21 additions & 3 deletions tests/ui/closure-expected-type/expect-fn-supply-fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -29,23 +38,32 @@ LL | with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
found fn pointer `for<'a> fn(&'a _)`

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 _)`
found fn pointer `fn(&_)`

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 _)`
found fn pointer `fn(&_)`

error: aborting due to 5 previous errors
error: aborting due to 7 previous errors

For more information about this error, try `rustc --explain E0308`.
20 changes: 10 additions & 10 deletions tests/ui/coherence/coherence-fn-covariant-bound-vs-static.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
// Test that impls for these two types are considered ovelapping:
//@ check-pass

// These types were previously considered equal as they are subtypes of each other.
// This has been changed in #118247 and we now consider them to be disjoint.
//
// In our test:
//
// * `for<'r> fn(fn(&'r u32))`
// * `fn(fn(&'a u32)` where `'a` is free
//
// This is because, for `'a = 'static`, the two types overlap.
// Effectively for them to be equal to you get:
// These were considered equal as for `'a = 'static` subtyping succeeds in both
// directions:
//
// * `for<'r> fn(fn(&'r u32)) <: fn(fn(&'static u32))`
// * true if `exists<'r> { 'r: 'static }` (obviously true)
Expand All @@ -15,12 +20,7 @@ trait Trait {}

impl Trait for for<'r> fn(fn(&'r ())) {}
impl<'a> Trait for fn(fn(&'a ())) {}
//~^ ERROR conflicting implementations
//
// 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
// "error" when these two types are equated.
//
// [MCP 295]: https://github.com/rust-lang/compiler-team/issues/295
//~^ WARN conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))` [coherence_leak_check]
//~| WARN the behavior may change in a future release

fn main() {}
10 changes: 6 additions & 4 deletions tests/ui/coherence/coherence-fn-covariant-bound-vs-static.stderr
Original file line number Diff line number Diff line change
@@ -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:22: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: the behavior may change 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`.
12 changes: 7 additions & 5 deletions tests/ui/coherence/coherence-fn-inputs.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
// Test that we consider these two types completely equal:
//@ check-pass

// These types were previously considered equal as they are subtypes of each other.
// This has been changed in #118247 and we now consider them to be disjoint.
//
// * `for<'a, 'b> fn(&'a u32, &'b u32)`
// * `for<'c> fn(&'c u32, &'c u32)`
//
// For a long time we considered these to be distinct types. But in fact they
// are equivalent, if you work through the implications of subtyping -- this is
// because:
// These types are subtypes of each other as:
//
// * `'c` can be the intersection of `'a` and `'b` (and there is always an intersection)
// * `'a` and `'b` can both be equal to `'c`

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 the behavior may change 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
Expand Down
Loading