Skip to content

Commit 1255ae4

Browse files
committed
leak check closure to fnptr non-lub-coercions too
1 parent a07ed70 commit 1255ae4

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
@@ -123,7 +123,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
123123

124124
fn unify_raw(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
125125
debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub);
126-
self.commit_if_ok(|_| {
126+
self.commit_if_ok(|snapshot| {
127+
let outer_universe = self.infcx.universe();
128+
127129
let at = self.at(&self.cause, self.fcx.param_env);
128130

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

@@ -751,40 +773,26 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
751773
b: ty::PolyFnSig<'tcx>,
752774
adjustment: Option<Adjust>,
753775
) -> CoerceResult<'tcx> {
754-
self.commit_if_ok(|snapshot| {
755-
let outer_universe = self.infcx.universe();
756-
757-
let a_ty = Ty::new_fn_ptr(self.tcx, a);
758-
let b_ty = Ty::new_fn_ptr(self.tcx, b);
776+
let a_ty = Ty::new_fn_ptr(self.tcx, a);
777+
let b_ty = Ty::new_fn_ptr(self.tcx, b);
759778

760-
// Handle coercing `fn(..)` to `unsafe fn(..)`
761-
let result = if a.safety().is_safe() && b.safety().is_unsafe() {
762-
let unsafe_a_ty = self.tcx.safe_to_unsafe_fn_ty(a);
763-
let adjustment = adjustment.map(|kind| Adjustment { kind, target: a_ty });
779+
// Handle coercing `fn(..)` to `unsafe fn(..)`
780+
if a.safety().is_safe() && b.safety().is_unsafe() {
781+
let unsafe_a_ty = self.tcx.safe_to_unsafe_fn_ty(a);
782+
let adjustment = adjustment.map(|kind| Adjustment { kind, target: a_ty });
764783

765-
self.unify_and(
766-
unsafe_a_ty,
767-
b_ty,
768-
adjustment,
769-
Adjust::Pointer(PointerCoercion::UnsafeFnPointer),
770-
)
771-
} else {
772-
match adjustment {
773-
Some(adjust) => self.unify_and(a_ty, b_ty, [], adjust),
774-
None => self.unify(a_ty, b_ty),
775-
}
776-
};
777-
778-
// FIXME(#73154): This is a hack. Currently LUB can generate
779-
// unsolvable constraints. Additionally, it returns `a`
780-
// unconditionally, even when the "LUB" is `b`. In the future, we
781-
// want the coerced type to be the actual supertype of these two,
782-
// but for now, we want to just error to ensure we don't lock
783-
// ourselves into a specific behavior with NLL.
784-
self.leak_check(outer_universe, Some(snapshot))?;
785-
786-
result
787-
})
784+
self.unify_and(
785+
unsafe_a_ty,
786+
b_ty,
787+
adjustment,
788+
Adjust::Pointer(PointerCoercion::UnsafeFnPointer),
789+
)
790+
} else {
791+
match adjustment {
792+
Some(adjust) => self.unify_and(a_ty, b_ty, [], adjust),
793+
None => self.unify(a_ty, b_ty),
794+
}
795+
}
788796
}
789797

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