Skip to content

Commit 50a11d1

Browse files
committed
leak check closure to fnptr non-lub-coercions too
1 parent 7d71b34 commit 50a11d1

15 files changed

+237
-90
lines changed

compiler/rustc_hir_typeck/src/coercion.rs

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
125125

126126
fn unify_raw(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
127127
debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub);
128-
self.commit_if_ok(|_| {
128+
self.commit_if_ok(|snapshot| {
129+
let outer_universe = self.infcx.universe();
130+
129131
let at = self.at(&self.cause, self.fcx.param_env);
130132

131133
let res = if self.use_lub {
@@ -138,7 +140,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
138140
// In the new solver, lazy norm may allow us to shallowly equate
139141
// more types, but we emit possibly impossible-to-satisfy obligations.
140142
// Filter these cases out to make sure our coercion is more accurate.
141-
match res {
143+
let res = match res {
142144
Ok(InferOk { value, obligations }) if self.next_trait_solver() => {
143145
let ocx = ObligationCtxt::new(self);
144146
ocx.register_obligations(obligations);
@@ -149,7 +151,27 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
149151
}
150152
}
151153
res => res,
152-
}
154+
};
155+
156+
// We leak check here mostly because lub operations are
157+
// kind of scuffed around binders. Instead of computing an actual
158+
// lub'd binder we instead:
159+
// - Equate the binders
160+
// - Return the lhs of the lub operation
161+
//
162+
// In order to actually ensure that equating the binders *does*
163+
// result in equal binders, and that the lhs is actually a supertype
164+
// of the rhs, we must perform a leak check here.
165+
//
166+
// Leak checking even when `use_lub` is false causes us to emit some
167+
// errors in dead code where borrowck would otherwise not catch the
168+
// subtyping errors. This is not soundness critical.
169+
//
170+
// FIXME: Ideally the actual `eq/sub/lub` would handle leak checks
171+
// themselves whenever a binder is entered.
172+
self.leak_check(outer_universe, Some(snapshot))?;
173+
174+
res
153175
})
154176
}
155177

@@ -805,40 +827,26 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
805827
b: ty::PolyFnSig<'tcx>,
806828
adjustment: Option<Adjust>,
807829
) -> CoerceResult<'tcx> {
808-
self.commit_if_ok(|snapshot| {
809-
let outer_universe = self.infcx.universe();
810-
811-
let a_ty = Ty::new_fn_ptr(self.tcx, a);
812-
let b_ty = Ty::new_fn_ptr(self.tcx, b);
830+
let a_ty = Ty::new_fn_ptr(self.tcx, a);
831+
let b_ty = Ty::new_fn_ptr(self.tcx, b);
813832

814-
// Handle coercing `fn(..)` to `unsafe fn(..)`
815-
let result = if a.safety().is_safe() && b.safety().is_unsafe() {
816-
let unsafe_a_ty = self.tcx.safe_to_unsafe_fn_ty(a);
817-
let adjustment = adjustment.map(|kind| Adjustment { kind, target: a_ty });
833+
// Handle coercing `fn(..)` to `unsafe fn(..)`
834+
if a.safety().is_safe() && b.safety().is_unsafe() {
835+
let unsafe_a_ty = self.tcx.safe_to_unsafe_fn_ty(a);
836+
let adjustment = adjustment.map(|kind| Adjustment { kind, target: a_ty });
818837

819-
self.unify_and(
820-
unsafe_a_ty,
821-
b_ty,
822-
adjustment,
823-
Adjust::Pointer(PointerCoercion::UnsafeFnPointer),
824-
)
825-
} else {
826-
match adjustment {
827-
Some(adjust) => self.unify_and(a_ty, b_ty, [], adjust),
828-
None => self.unify(a_ty, b_ty),
829-
}
830-
};
831-
832-
// FIXME(#73154): This is a hack. Currently LUB can generate
833-
// unsolvable constraints. Additionally, it returns `a`
834-
// unconditionally, even when the "LUB" is `b`. In the future, we
835-
// want the coerced type to be the actual supertype of these two,
836-
// but for now, we want to just error to ensure we don't lock
837-
// ourselves into a specific behavior with NLL.
838-
self.leak_check(outer_universe, Some(snapshot))?;
839-
840-
result
841-
})
838+
self.unify_and(
839+
unsafe_a_ty,
840+
b_ty,
841+
adjustment,
842+
Adjust::Pointer(PointerCoercion::UnsafeFnPointer),
843+
)
844+
} else {
845+
match adjustment {
846+
Some(adjust) => self.unify_and(a_ty, b_ty, [], adjust),
847+
None => self.unify(a_ty, b_ty),
848+
}
849+
}
842850
}
843851

844852
fn coerce_from_fn_pointer(
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
struct Foo<T>(T);
2+
3+
fn mk<T>() -> T {
4+
loop {}
5+
}
6+
7+
fn lub_deep_binder() {
8+
loop {}
9+
let a: Foo<for<'a> fn(&'a ())> = mk::<Foo<fn(&'static ())>>();
10+
//~^ ERROR: mismatched types
11+
}
12+
13+
fn main() {}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/leak_check_normal_sub.rs:9:38
3+
|
4+
LL | let a: Foo<for<'a> fn(&'a ())> = mk::<Foo<fn(&'static ())>>();
5+
| ----------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
6+
| |
7+
| expected due to this
8+
|
9+
= note: expected struct `Foo<for<'a> fn(&'a ())>`
10+
found struct `Foo<fn(&'static ())>`
11+
12+
error: aborting due to 1 previous error
13+
14+
For more information about this error, try `rustc --explain E0308`.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/leak_check_single_to_fnptr.rs:25:39
3+
|
4+
LL | let _target: for<'a> fn(&'a ()) = a_fndef;
5+
| ------------------ ^^^^^^^ one type is more general than the other
6+
| |
7+
| expected due to this
8+
|
9+
= note: expected fn pointer `for<'a> fn(&'a ())`
10+
found fn item `fn(&'static ()) {static_fndef}`
11+
12+
error[E0308]: mismatched types
13+
--> $DIR/leak_check_single_to_fnptr.rs:28:39
14+
|
15+
LL | let _target: for<'a> fn(&'a ()) = a_fnptr;
16+
| ------------------ ^^^^^^^ one type is more general than the other
17+
| |
18+
| expected due to this
19+
|
20+
= note: expected fn pointer `for<'a> fn(&'a ())`
21+
found fn pointer `fn(&())`
22+
23+
error[E0308]: mismatched types
24+
--> $DIR/leak_check_single_to_fnptr.rs:31:39
25+
|
26+
LL | let a_closure = |_: &'static ()| {};
27+
| ---------------- the found closure
28+
...
29+
LL | let _target: for<'a> fn(&'a ()) = a_closure;
30+
| ------------------ ^^^^^^^^^ one type is more general than the other
31+
| |
32+
| expected due to this
33+
|
34+
= note: expected fn pointer `for<'a> fn(&'a ())`
35+
found closure `{closure@$DIR/leak_check_single_to_fnptr.rs:21:21: 21:37}`
36+
= note: closure has signature: `fn(&'static ())`
37+
38+
error: aborting due to 3 previous errors
39+
40+
For more information about this error, try `rustc --explain E0308`.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error[E0308]: mismatched types
2+
--> $DIR/leak_check_single_to_fnptr.rs:25:39
3+
|
4+
LL | let _target: for<'a> fn(&'a ()) = a_fndef;
5+
| ------------------ ^^^^^^^ one type is more general than the other
6+
| |
7+
| expected due to this
8+
|
9+
= note: expected fn pointer `for<'a> fn(&'a ())`
10+
found fn item `fn(&'static ()) {static_fndef}`
11+
12+
error[E0308]: mismatched types
13+
--> $DIR/leak_check_single_to_fnptr.rs:28:39
14+
|
15+
LL | let _target: for<'a> fn(&'a ()) = a_fnptr;
16+
| ------------------ ^^^^^^^ one type is more general than the other
17+
| |
18+
| expected due to this
19+
|
20+
= note: expected fn pointer `for<'a> fn(&'a ())`
21+
found fn pointer `fn(&())`
22+
23+
error[E0308]: mismatched types
24+
--> $DIR/leak_check_single_to_fnptr.rs:31:39
25+
|
26+
LL | let a_closure = |_: &'static ()| {};
27+
| ---------------- the found closure
28+
...
29+
LL | let _target: for<'a> fn(&'a ()) = a_closure;
30+
| ------------------ ^^^^^^^^^ one type is more general than the other
31+
| |
32+
| expected due to this
33+
|
34+
= note: expected fn pointer `for<'a> fn(&'a ())`
35+
found closure `{closure@$DIR/leak_check_single_to_fnptr.rs:21:21: 21:37}`
36+
= note: closure has signature: `fn(&'static ())`
37+
38+
error: aborting due to 3 previous errors
39+
40+
For more information about this error, try `rustc --explain E0308`.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
//@ revisions: livecode deadcode
2+
3+
// When coercing to a fnptr check that we leak check when relating
4+
// the signatures. Previously we only leak checked for fnptr/fndef
5+
// to fnptr coercions, but not closure to fnptr coercions. This
6+
// resulted in closure to fnptr coercions not erroring in dead code
7+
// when equivalent code with fndefs/fnptrs would error.
8+
//
9+
// Outside of dead code all cases wind up erroring in borrowck so this
10+
// is not a soundness concern.
11+
12+
fn mk<T>() -> T {
13+
loop {}
14+
}
15+
16+
fn static_fndef(_: &'static ()) {}
17+
18+
fn one_way_coerce() {
19+
let a_fndef = static_fndef;
20+
let a_fnptr = mk::<fn(&'static ())>();
21+
let a_closure = |_: &'static ()| {};
22+
23+
#[cfg(deadcode)]
24+
loop {}
25+
let _target: for<'a> fn(&'a ()) = a_fndef;
26+
//[livecode]~^ ERROR: mismatched types
27+
//[deadcode]~^^ ERROR: mismatched types
28+
let _target: for<'a> fn(&'a ()) = a_fnptr;
29+
//[livecode]~^ ERROR: mismatched types
30+
//[deadcode]~^^ ERROR: mismatched types
31+
let _target: for<'a> fn(&'a ()) = a_closure;
32+
//[livecode]~^ ERROR: mismatched types
33+
//[deadcode]~^^ ERROR: mismatched types
34+
}
35+
36+
fn main() {}

tests/ui/const-generics/invariant.stderr

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ LL | impl SadBee for fn(&'static ()) {
1515
error[E0308]: mismatched types
1616
--> $DIR/invariant.rs:25:5
1717
|
18+
LL | fn covariant(v: &'static Foo<for<'a> fn(&'a ())>) -> &'static Foo<fn(&'static ())> {
19+
| ----------------------------- expected `&'static Foo<fn(&'static ())>` because of return type
1820
LL | v
1921
| ^ one type is more general than the other
2022
|
21-
= note: expected reference `&Foo<fn(&())>`
22-
found reference `&Foo<for<'a> fn(&'a ())>`
23+
= note: expected reference `&'static Foo<fn(&'static ())>`
24+
found reference `&'static Foo<for<'a> fn(&'a ())>`
2325

2426
error: aborting due to 1 previous error; 1 warning emitted
2527

tests/ui/higher-ranked/higher-ranked-lifetime-equality.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,5 @@ fn main() {
3333
foo.deref();
3434
let foo: Foo<Two> = foo;
3535
//~^ ERROR mismatched types [E0308]
36-
//~| ERROR mismatched types [E0308]
3736
foo.deref();
3837
}

tests/ui/higher-ranked/higher-ranked-lifetime-equality.stderr

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,13 @@ error[E0308]: mismatched types
22
--> $DIR/higher-ranked-lifetime-equality.rs:34:25
33
|
44
LL | let foo: Foo<Two> = foo;
5-
| ^^^ one type is more general than the other
5+
| -------- ^^^ one type is more general than the other
6+
| |
7+
| expected due to this
68
|
79
= note: expected struct `my_api::Foo<for<'a, 'b> fn(&'a (), &'b ())>`
810
found struct `my_api::Foo<for<'a> fn(&'a (), &'a ())>`
911

10-
error[E0308]: mismatched types
11-
--> $DIR/higher-ranked-lifetime-equality.rs:34:25
12-
|
13-
LL | let foo: Foo<Two> = foo;
14-
| ^^^ one type is more general than the other
15-
|
16-
= note: expected struct `my_api::Foo<for<'a, 'b> fn(&'a (), &'b ())>`
17-
found struct `my_api::Foo<for<'a> fn(&'a (), &'a ())>`
18-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
19-
20-
error: aborting due to 2 previous errors
12+
error: aborting due to 1 previous error
2113

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

tests/ui/higher-ranked/leak-check/candidate-from-env-universe-err-project.next.stderr

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,19 @@ note: required by a bound in `projection_bound`
1010
LL | fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
1111
| ^^^^^^^^^^^^^ required by this bound in `projection_bound`
1212

13-
error: higher-ranked subtype error
14-
--> $DIR/candidate-from-env-universe-err-project.rs:52:30
13+
error[E0308]: mismatched types
14+
--> $DIR/candidate-from-env-universe-err-project.rs:52:68
1515
|
1616
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
17-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18-
19-
error: higher-ranked subtype error
20-
--> $DIR/candidate-from-env-universe-err-project.rs:52:30
21-
|
22-
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
23-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
| ----------------------------------- ^^^^^^ one type is more general than the other
18+
| |
19+
| expected due to this
2420
|
25-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
21+
= note: expected fn pointer `fn(usize)`
22+
found closure `{closure@$DIR/candidate-from-env-universe-err-project.rs:52:68: 52:71}`
23+
= note: closure has signature: `fn(usize)`
2624

27-
error: aborting due to 3 previous errors
25+
error: aborting due to 2 previous errors
2826

29-
For more information about this error, try `rustc --explain E0271`.
27+
Some errors have detailed explanations: E0271, E0308.
28+
For more information about an error, try `rustc --explain E0271`.

0 commit comments

Comments
 (0)