Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit ef93bd5

Browse files
borsbors
committed
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`
2 parents 7a34091 + 0fd80ba commit ef93bd5

23 files changed

+316
-88
lines changed

‎compiler/rustc_hir_analysis/src/check/intrinsic.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
138138
let name_str = intrinsic_name.as_str();
139139

140140
let bound_vars = tcx.mk_bound_variable_kinds(&[
141+
ty::BoundVariableKind::Region(ty::BrAnon),
141142
ty::BoundVariableKind::Region(ty::BrAnon),
142143
ty::BoundVariableKind::Region(ty::BrEnv),
143144
]);
@@ -151,7 +152,7 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
151152
let env_region = ty::Region::new_bound(
152153
tcx,
153154
ty::INNERMOST,
154-
ty::BoundRegion { var: ty::BoundVar::from_u32(1), kind: ty::BrEnv },
155+
ty::BoundRegion { var: ty::BoundVar::from_u32(2), kind: ty::BrEnv },
155156
);
156157
let va_list_ty = tcx.type_of(did).instantiate(tcx, &[region.into()]);
157158
(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<'_>) {
446447

447448
sym::raw_eq => {
448449
let br = ty::BoundRegion { var: ty::BoundVar::from_u32(0), kind: ty::BrAnon };
449-
let param_ty =
450+
let param_ty_lhs =
451+
Ty::new_imm_ref(tcx, ty::Region::new_bound(tcx, ty::INNERMOST, br), param(0));
452+
let br = ty::BoundRegion { var: ty::BoundVar::from_u32(1), kind: ty::BrAnon };
453+
let param_ty_rhs =
450454
Ty::new_imm_ref(tcx, ty::Region::new_bound(tcx, ty::INNERMOST, br), param(0));
451-
(1, vec![param_ty; 2], tcx.types.bool)
455+
(1, vec![param_ty_lhs, param_ty_rhs], tcx.types.bool)
452456
}
453457

454458
sym::black_box => (1, vec![param(0)], param(0)),

‎compiler/rustc_infer/src/infer/equate.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
165165
}
166166

167167
if a.skip_binder().has_escaping_bound_vars() || b.skip_binder().has_escaping_bound_vars() {
168-
self.fields.higher_ranked_sub(a, b, self.a_is_expected)?;
169-
self.fields.higher_ranked_sub(b, a, self.a_is_expected)?;
168+
self.fields.higher_ranked_equate(a, b, self.a_is_expected)?;
169+
self.fields.higher_ranked_equate(b, a, !self.a_is_expected)?;
170170
} else {
171171
// Fast path for the common case.
172172
self.relate(a.skip_binder(), b.skip_binder())?;

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,40 @@ impl<'a, 'tcx> CombineFields<'a, 'tcx> {
5757
// placeholders which **must not** be named afterwards.
5858
Ok(())
5959
}
60+
61+
#[instrument(skip(self), level = "debug")]
62+
pub fn higher_ranked_equate<T>(
63+
&mut self,
64+
a: Binder<'tcx, T>,
65+
b: Binder<'tcx, T>,
66+
a_is_expected: bool,
67+
) -> RelateResult<'tcx, ()>
68+
where
69+
T: Relate<'tcx>,
70+
{
71+
let span = self.trace.cause.span;
72+
// First, we instantiate each bound region in the supertype with a
73+
// fresh placeholder region. Note that this automatically creates
74+
// a new universe if needed.
75+
let b_prime = self.infcx.instantiate_binder_with_placeholders(b);
76+
77+
// Next, we instantiate each bound region in the subtype
78+
// with a fresh region variable. These region variables --
79+
// but no other preexisting region variables -- can name
80+
// the placeholders.
81+
let a_prime = self.infcx.instantiate_binder_with_fresh_vars(span, HigherRankedType, a);
82+
83+
debug!("a_prime={:?}", a_prime);
84+
debug!("b_prime={:?}", b_prime);
85+
86+
// Compare types now that bound regions have been replaced.
87+
let result = self.equate(a_is_expected).relate(a_prime, b_prime)?;
88+
89+
debug!("OK result={result:?}");
90+
// NOTE: returning the result here would be dangerous as it contains
91+
// placeholders which **must not** be named afterwards.
92+
Ok(())
93+
}
6094
}
6195

6296
impl<'tcx> InferCtxt<'tcx> {

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

Lines changed: 67 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -595,69 +595,84 @@ where
595595
// - Instantiate binders on `b` universally, yielding a universe U1.
596596
// - Instantiate binders on `a` existentially in U1.
597597

598-
debug!(?self.ambient_variance);
599-
600598
if let (Some(a), Some(b)) = (a.no_bound_vars(), b.no_bound_vars()) {
601599
// Fast path for the common case.
602600
self.relate(a, b)?;
603601
return Ok(ty::Binder::dummy(a));
604602
}
605603

606-
if self.ambient_covariance() {
607-
// Covariance, so we want `for<..> A <: for<..> B` --
608-
// therefore we compare any instantiation of A (i.e., A
609-
// instantiated with existentials) against every
610-
// instantiation of B (i.e., B instantiated with
611-
// universals).
612-
613-
// Reset the ambient variance to covariant. This is needed
614-
// to correctly handle cases like
615-
//
616-
// for<'a> fn(&'a u32, &'a u32) == for<'b, 'c> fn(&'b u32, &'c u32)
617-
//
618-
// Somewhat surprisingly, these two types are actually
619-
// **equal**, even though the one on the right looks more
620-
// polymorphic. The reason is due to subtyping. To see it,
621-
// consider that each function can call the other:
622-
//
623-
// - The left function can call the right with `'b` and
624-
// `'c` both equal to `'a`
625-
//
626-
// - The right function can call the left with `'a` set to
627-
// `{P}`, where P is the point in the CFG where the call
628-
// itself occurs. Note that `'b` and `'c` must both
629-
// include P. At the point, the call works because of
630-
// subtyping (i.e., `&'b u32 <: &{P} u32`).
631-
let variance = std::mem::replace(&mut self.ambient_variance, ty::Variance::Covariant);
632-
633-
// Note: the order here is important. Create the placeholders first, otherwise
634-
// we assign the wrong universe to the existential!
635-
let b_replaced = self.instantiate_binder_with_placeholders(b);
636-
let a_replaced = self.instantiate_binder_with_existentials(a);
637-
638-
self.relate(a_replaced, b_replaced)?;
639-
640-
self.ambient_variance = variance;
641-
}
604+
match self.ambient_variance {
605+
ty::Variance::Covariant => {
606+
// Covariance, so we want `for<..> A <: for<..> B` --
607+
// therefore we compare any instantiation of A (i.e., A
608+
// instantiated with existentials) against every
609+
// instantiation of B (i.e., B instantiated with
610+
// universals).
611+
612+
// Reset the ambient variance to covariant. This is needed
613+
// to correctly handle cases like
614+
//
615+
// for<'a> fn(&'a u32, &'a u32) == for<'b, 'c> fn(&'b u32, &'c u32)
616+
//
617+
// Somewhat surprisingly, these two types are actually
618+
// **equal**, even though the one on the right looks more
619+
// polymorphic. The reason is due to subtyping. To see it,
620+
// consider that each function can call the other:
621+
//
622+
// - The left function can call the right with `'b` and
623+
// `'c` both equal to `'a`
624+
//
625+
// - The right function can call the left with `'a` set to
626+
// `{P}`, where P is the point in the CFG where the call
627+
// itself occurs. Note that `'b` and `'c` must both
628+
// include P. At the point, the call works because of
629+
// subtyping (i.e., `&'b u32 <: &{P} u32`).
630+
let variance =
631+
std::mem::replace(&mut self.ambient_variance, ty::Variance::Covariant);
632+
633+
// Note: the order here is important. Create the placeholders first, otherwise
634+
// we assign the wrong universe to the existential!
635+
let b_replaced = self.instantiate_binder_with_placeholders(b);
636+
let a_replaced = self.instantiate_binder_with_existentials(a);
637+
638+
self.relate(a_replaced, b_replaced)?;
639+
640+
self.ambient_variance = variance;
641+
}
642642

643-
if self.ambient_contravariance() {
644-
// Contravariance, so we want `for<..> A :> for<..> B`
645-
// -- therefore we compare every instantiation of A (i.e.,
646-
// A instantiated with universals) against any
647-
// instantiation of B (i.e., B instantiated with
648-
// existentials). Opposite of above.
643+
ty::Variance::Contravariant => {
644+
// Contravariance, so we want `for<..> A :> for<..> B`
645+
// -- therefore we compare every instantiation of A (i.e.,
646+
// A instantiated with universals) against any
647+
// instantiation of B (i.e., B instantiated with
648+
// existentials). Opposite of above.
649+
650+
// Reset ambient variance to contravariance. See the
651+
// covariant case above for an explanation.
652+
let variance =
653+
std::mem::replace(&mut self.ambient_variance, ty::Variance::Contravariant);
654+
655+
let a_replaced = self.instantiate_binder_with_placeholders(a);
656+
let b_replaced = self.instantiate_binder_with_existentials(b);
649657

650-
// Reset ambient variance to contravariance. See the
651-
// covariant case above for an explanation.
652-
let variance =
653-
std::mem::replace(&mut self.ambient_variance, ty::Variance::Contravariant);
658+
self.relate(a_replaced, b_replaced)?;
654659

655-
let a_replaced = self.instantiate_binder_with_placeholders(a);
656-
let b_replaced = self.instantiate_binder_with_existentials(b);
660+
self.ambient_variance = variance;
661+
}
662+
663+
ty::Variance::Invariant => {
664+
// Note: the order here is important. Create the placeholders first, otherwise
665+
// we assign the wrong universe to the existential!
666+
let b_replaced = self.instantiate_binder_with_placeholders(b);
667+
let a_replaced = self.instantiate_binder_with_existentials(a);
668+
self.relate(a_replaced, b_replaced)?;
657669

658-
self.relate(a_replaced, b_replaced)?;
670+
let a_replaced = self.instantiate_binder_with_placeholders(a);
671+
let b_replaced = self.instantiate_binder_with_existentials(b);
672+
self.relate(a_replaced, b_replaced)?;
673+
}
659674

660-
self.ambient_variance = variance;
675+
ty::Variance::Bivariant => {}
661676
}
662677

663678
Ok(a)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ impl<'a> Foo<fn(&'a ())> {
88
}
99

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

1414
fn main() {}

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
1-
error: higher-ranked subtype error
2-
--> $DIR/issue-111404-1.rs:10:1
1+
error[E0308]: mismatched types
2+
--> $DIR/issue-111404-1.rs:10:11
33
|
44
LL | fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
6+
|
7+
= note: expected struct `Foo<fn(&())>`
8+
found struct `Foo<for<'b> fn(&'b ())>`
69

7-
error: higher-ranked subtype error
8-
--> $DIR/issue-111404-1.rs:10:1
10+
error[E0308]: mismatched types
11+
--> $DIR/issue-111404-1.rs:10:11
912
|
1013
LL | fn bar(_: fn(Foo<for<'b> fn(Foo<fn(&'b ())>::Assoc)>::Assoc)) {}
11-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
1215
|
16+
= note: expected struct `Foo<fn(&())>`
17+
found struct `Foo<for<'b> fn(&'b ())>`
1318
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
1419

1520
error: aborting due to 2 previous errors
1621

22+
For more information about this error, try `rustc --explain E0308`.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,16 @@ fn expect_free_supply_bound() {
3030
// Here, we are given a function whose region is bound at closure level,
3131
// but we expect one bound in the argument. Error results.
3232
with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
33-
//~^ ERROR mismatched types
33+
//~^ ERROR mismatched types [E0308]
34+
//~| ERROR lifetime may not live long enough
3435
}
3536

3637
fn expect_bound_supply_free_from_fn<'x>(x: &'x u32) {
3738
// Here, we are given a `fn(&u32)` but we expect a `fn(&'x
3839
// u32)`. In principle, this could be ok, but we demand equality.
3940
with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
40-
//~^ ERROR mismatched types
41+
//~^ ERROR mismatched types [E0308]
42+
//~| ERROR lifetime may not live long enough
4143
}
4244

4345
fn expect_bound_supply_free_from_closure() {

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ LL | fn expect_free_supply_free_from_fn<'x>(x: &'x u32) {
1919
LL | with_closure_expecting_fn_with_free_region(|x: fn(&'x u32), y| {});
2020
| ^ requires that `'x` must outlive `'static`
2121

22+
error: lifetime may not live long enough
23+
--> $DIR/expect-fn-supply-fn.rs:32:49
24+
|
25+
LL | with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
26+
| ^
27+
| |
28+
| has type `fn(&'1 u32)`
29+
| requires that `'1` must outlive `'static`
30+
2231
error[E0308]: mismatched types
2332
--> $DIR/expect-fn-supply-fn.rs:32:49
2433
|
@@ -29,23 +38,32 @@ LL | with_closure_expecting_fn_with_free_region(|x: fn(&u32), y| {});
2938
found fn pointer `for<'a> fn(&'a u32)`
3039

3140
error[E0308]: mismatched types
32-
--> $DIR/expect-fn-supply-fn.rs:39:50
41+
--> $DIR/expect-fn-supply-fn.rs:40:50
3342
|
3443
LL | with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
3544
| ^ one type is more general than the other
3645
|
3746
= note: expected fn pointer `for<'a> fn(&'a u32)`
3847
found fn pointer `fn(&u32)`
3948

49+
error: lifetime may not live long enough
50+
--> $DIR/expect-fn-supply-fn.rs:40:50
51+
|
52+
LL | fn expect_bound_supply_free_from_fn<'x>(x: &'x u32) {
53+
| -- lifetime `'x` defined here
54+
...
55+
LL | with_closure_expecting_fn_with_bound_region(|x: fn(&'x u32), y| {});
56+
| ^ requires that `'x` must outlive `'static`
57+
4058
error[E0308]: mismatched types
41-
--> $DIR/expect-fn-supply-fn.rs:48:50
59+
--> $DIR/expect-fn-supply-fn.rs:50:50
4260
|
4361
LL | with_closure_expecting_fn_with_bound_region(|x: Foo<'_>, y| {
4462
| ^ one type is more general than the other
4563
|
4664
= note: expected fn pointer `for<'a> fn(&'a u32)`
4765
found fn pointer `fn(&u32)`
4866

49-
error: aborting due to 5 previous errors
67+
error: aborting due to 7 previous errors
5068

5169
For more information about this error, try `rustc --explain E0308`.

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// check-pass
2+
13
// Test that impls for these two types are considered ovelapping:
24
//
35
// * `for<'r> fn(fn(&'r u32))`
@@ -15,7 +17,8 @@ trait Trait {}
1517

1618
impl Trait for for<'r> fn(fn(&'r ())) {}
1719
impl<'a> Trait for fn(fn(&'a ())) {}
18-
//~^ ERROR conflicting implementations
20+
//~^ WARN conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))` [coherence_leak_check]
21+
//~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
1922
//
2023
// Note in particular that we do NOT get a future-compatibility warning
2124
// here. This is because the new leak-check proposed in [MCP 295] does not

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1-
error[E0119]: conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))`
2-
--> $DIR/coherence-fn-covariant-bound-vs-static.rs:17:1
1+
warning: conflicting implementations of trait `Trait` for type `for<'r> fn(fn(&'r ()))`
2+
--> $DIR/coherence-fn-covariant-bound-vs-static.rs:19:1
33
|
44
LL | impl Trait for for<'r> fn(fn(&'r ())) {}
55
| ------------------------------------- first implementation here
66
LL | impl<'a> Trait for fn(fn(&'a ())) {}
77
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `for<'r> fn(fn(&'r ()))`
88
|
9+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
10+
= note: for more information, see issue #56105 <https://github.com/rust-lang/rust/issues/56105>
911
= note: this behavior recently changed as a result of a bug fix; see rust-lang/rust#56105 for details
12+
= note: `#[warn(coherence_leak_check)]` on by default
1013

11-
error: aborting due to 1 previous error
14+
warning: 1 warning emitted
1215

13-
For more information about this error, try `rustc --explain E0119`.

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

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

0 commit comments

Comments
 (0)