Skip to content

Commit ae9a3aa

Browse files
committed
leak check closure to fnptr non-lub-coercions too
1 parent 68806a5 commit ae9a3aa

File tree

4 files changed

+159
-35
lines changed

4 files changed

+159
-35
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

@@ -780,40 +802,26 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
780802
b: ty::PolyFnSig<'tcx>,
781803
adjustment: Option<Adjust>,
782804
) -> CoerceResult<'tcx> {
783-
self.commit_if_ok(|snapshot| {
784-
let outer_universe = self.infcx.universe();
785-
786-
let a_ty = Ty::new_fn_ptr(self.tcx, a);
787-
let b_ty = Ty::new_fn_ptr(self.tcx, b);
805+
let a_ty = Ty::new_fn_ptr(self.tcx, a);
806+
let b_ty = Ty::new_fn_ptr(self.tcx, b);
788807

789-
// Handle coercing `fn(..)` to `unsafe fn(..)`
790-
let result = if a.safety().is_safe() && b.safety().is_unsafe() {
791-
let unsafe_a_ty = self.tcx.safe_to_unsafe_fn_ty(a);
792-
let adjustment = adjustment.map(|kind| Adjustment { kind, target: a_ty });
808+
// Handle coercing `fn(..)` to `unsafe fn(..)`
809+
if a.safety().is_safe() && b.safety().is_unsafe() {
810+
let unsafe_a_ty = self.tcx.safe_to_unsafe_fn_ty(a);
811+
let adjustment = adjustment.map(|kind| Adjustment { kind, target: a_ty });
793812

794-
self.unify_and(
795-
unsafe_a_ty,
796-
b_ty,
797-
adjustment,
798-
Adjust::Pointer(PointerCoercion::UnsafeFnPointer),
799-
)
800-
} else {
801-
match adjustment {
802-
Some(adjust) => self.unify_and(a_ty, b_ty, [], adjust),
803-
None => self.unify(a_ty, b_ty),
804-
}
805-
};
806-
807-
// FIXME(#73154): This is a hack. Currently LUB can generate
808-
// unsolvable constraints. Additionally, it returns `a`
809-
// unconditionally, even when the "LUB" is `b`. In the future, we
810-
// want the coerced type to be the actual supertype of these two,
811-
// but for now, we want to just error to ensure we don't lock
812-
// ourselves into a specific behavior with NLL.
813-
self.leak_check(outer_universe, Some(snapshot))?;
814-
815-
result
816-
})
813+
self.unify_and(
814+
unsafe_a_ty,
815+
b_ty,
816+
adjustment,
817+
Adjust::Pointer(PointerCoercion::UnsafeFnPointer),
818+
)
819+
} else {
820+
match adjustment {
821+
Some(adjust) => self.unify_and(a_ty, b_ty, [], adjust),
822+
None => self.unify(a_ty, b_ty),
823+
}
824+
}
817825
}
818826

819827
fn coerce_from_fn_pointer(
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() {}

0 commit comments

Comments
 (0)