Skip to content

Commit 9eb03e5

Browse files
authored
Rollup merge of #150316 - ShoyuVanilla:fudge-checks, r=lcnr
Do not use non-wf input expectations from fudge when checking function calls cc #149379 r? lcnr # FCP: Do not use non-wf input expectations from fudge when checking function calls ## What is fudging? https://github.com/rust-lang/rust/blob/71e00273c0921e1bc850ae8cc4161fbb44cfa848/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L168-L170 Consider this coercion site: `let _: Box<dyn Fn(&str) -> usize> = Box::new(|s| s.len());` We rely on the expectation to eagerly infer the type of `s` to be `&str`. However, `dyn Fn(&str) -> usize` is not a valid type as the argument of `Box::new`, as it is not `Sized`. *Fudging* is the mechanism we use to propagate the expectation through the `Box::new` call without constraining its generic parameter. Fudging computes the expected argument types by acting as if we're able to propagate the expected return type directly through the function, without any coercions on the return site. Given that we may actually want to coerce afterwards, we cannot actually commit any constraints here. We therefore compute the expectations for the function arguments in a `probe` and rely on *fudging* to be able to name any inference variables created inside of the probe. After the fudging step, we weaken the resulting expectation if it is an unsized type in the following lines: https://github.com/rust-lang/rust/blob/71e00273c0921e1bc850ae8cc4161fbb44cfa848/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L354 https://github.com/rust-lang/rust/blob/71e00273c0921e1bc850ae8cc4161fbb44cfa848/compiler/rustc_hir_typeck/src/expectation.rs#L77-L89 Because function arguments must be `Sized`, this weakening prevents us from applying wrong, unsized coercions to them. ## How fudging currently goes wrong We have an opened issue for tracking such cases: #149379. Broadly, the failures seem to fall into two buckets. ### We may end up with non–well-formed expectations Fudging can produce an expected type that is not well-formed. That would eventually result in an error failing the well-formedness check, either when we do the coercion with the expected argument types, or when we select the remaining obligations. ```rust fn foo<T>(x: (T, ())) -> Box<T> { Box::new(x.0) } fn main() { // We use `(dyn Send, ())` as the expectation the argument. let _: Box<dyn Send> = foo(((), ())); } ``` ### Weakening fudged expectation is not covering all the cases ```rust fn field_to_box<T>(x: &(T,)) -> &T { &x.0 } fn main() { // `Expectation::rvalue_hint` only checks whether the whole argument // itself is `Sized`. It does not check whether the function requires // its generic parameters to be `Sized`. let _: &dyn Send = field_to_box(&(1,)); } ``` ## What this PR fixes ### One of the problematic cases of the issue This PR fixes the first case, by simply checking well-formedness of the each expected argument types inside the fudge scope. This is a reasonable change because: - Non well-formed expectation would result in a well-formedness error so not using such expectation wouldn't make any previously compiled code being not compiled anymore - Dropping a non well-formed expectation does not mean we stop providing expectations for argument coercions altogether. If fudging fails, we still fall back to using the types from the function signature as expectations in the usual path: https://github.com/rust-lang/rust/blob/71e00273c0921e1bc850ae8cc4161fbb44cfa848/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L330-L336 #### Related tests - Fixes [tests/ui/coercion/fudge-inference/fn-ret-trait-object-propagated-to-inputs-issue-149379-1.rs](5668ad5#diff-1468a6d8495f7adfb4a64508f002bb934c13d13871662de6efd60433649401fd) ### Limited for `-Znext-solver` Separately (and not directly tied to the above issue), this PR also fixes a next-solver regression tracked at: rust-lang/trait-system-refactor-initiative#259. That regression was introduced by #149320, which started normalizing expected function input types inside the fudge scope. Because all inference-variable relationships and pending obligations are dropped out when we exit the fudge scope, we drop the ambiguous goal used to normalize, leaving us with an unconstrained inference variable in the expectation. This PR fixes that by normalizing the expectation outside the fudge scope, so the resulting normalization obligations are preserved to the `InferCtxt`'s `ObligationCtxt`. #### Related tests - Fixes [tests/ui/traits/next-solver/fudge-inference/do-not-drop-ambig-normalization.rs](https://github.com/rust-lang/rust/pull/150316/files#diff-42e97f178fbdee7c3405ae12409eb0bca4eec92488971c703b26c083eadf728a) ## Does this PR break anything I don't think there should be any breakage affecting the old solver. The separate expectation normalization change only affecting the new solver does break an existing [test](https://github.com/rust-lang/rust/pull/150316/files#diff-3b946a09e063aad2a4fa6b0893508d5ffab78763b8465abfe1f689d349fda815). This is unfortunate but I think this change should be done because - The broken test also doesn't compile with the old solver - The expectation normalization change is necessary to compile stuff supported on stable ## What this PR doesn't fix This PR doesn't fix the second case -- *Weakening fudged expectation is not covering all the cases*. @lcnr has [suggested](https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/expectations.20from.20fudging.20are.20a.20mess.20.23149379/near/560625205) the following solution for that problem: > check whether a function where-bound errors without an out coercion, if so, weaken the expectation to `ExpectRvalueLikeUnsized` I experimented with this and it works well in many cases. However, on the old solver, checking where-bounds cannot reliably be treated as speculative: if we hit an overflow while checking the where-bounds, the old solver can fail the entire compilation rather than merely treating the check as failed and relaxing the expectation. Since this second class of issues affects both the old solver and the next-solver, it seems preferable to keep the conservative behavior for now, at least until the next-solver is stabilized, rather than introducing a next-solver-only relaxation that might create new regressions and complicate stabilization efforts. #### Related tests - Does NOT Fix [tests/ui/coercion/fudge-inference/expectated-input-not-satisfying-fn-bounds-issue-89299.rs](https://github.com/rust-lang/rust/pull/150316/files#diff-fdcfa8ab660c052dbe246db279d167ea8a309bfe10ca6163f7fa1836be2b30d6) - Does NOT Fix [tests/ui/coercion/fudge-inference/expectated-input-not-satisfying-fn-bounds-issue-149881.rs](https://github.com/rust-lang/rust/pull/150316/files#diff-1ccbb181cbf164841ca5af350ecf903c802a4854bda309e83e91c3b917809a55) - Does NOT Fix [tests/ui/coercion/fudge-inference/fn-ret-trait-object-propagated-to-inputs-issue-149379-3.rs](https://github.com/rust-lang/rust/pull/150316/files#diff-b12e01cc3c265db42f135d67425d8b2bd0d9c44c680b3e8c49d1f845a0b25d09)
2 parents d0442e2 + bec4304 commit 9eb03e5

27 files changed

Lines changed: 255 additions & 284 deletions

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
215215
self.check_place_expr_if_unsized(fn_input_ty, arg_expr);
216216
}
217217

218+
let formal_input_tys_ns;
219+
let formal_input_tys = if self.next_trait_solver() {
220+
// In the new solver, the normalizations are done lazily.
221+
// Because of this, if we encounter unnormalized alias types inside this
222+
// fudge scope, we might lose the relationships between them and other vars
223+
// when fudging inference variables created here.
224+
// So, we utilize generalization to normalize aliases by adding a new
225+
// inference var and equating it with the type we want to pull out of the
226+
// fudge scope.
227+
formal_input_tys_ns = formal_input_tys
228+
.iter()
229+
.map(|&ty| {
230+
let generalized_ty = self.next_ty_var(call_span);
231+
self.demand_eqtype(call_span, ty, generalized_ty);
232+
generalized_ty
233+
})
234+
.collect_vec();
235+
236+
formal_input_tys_ns.as_slice()
237+
} else {
238+
formal_input_tys
239+
};
240+
218241
// First, let's unify the formal method signature with the expectation eagerly.
219242
// We use this to guide coercion inference; it's output is "fudged" which means
220243
// any remaining type variables are assigned to new, unrelated variables. This
@@ -238,37 +261,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
238261
let origin = self.misc(call_span);
239262
ocx.sup(&origin, self.param_env, expected_output, formal_output)?;
240263

241-
let formal_input_tys_ns;
242-
let formal_input_tys = if self.next_trait_solver() {
243-
// In the new solver, the normalizations are done lazily.
244-
// Because of this, if we encounter unnormalized alias types inside this
245-
// fudge scope, we might lose the relationships between them and other vars
246-
// when fudging inference variables created here.
247-
// So, we utilize generalization to normalize aliases by adding a new
248-
// inference var and equating it with the type we want to pull out of the
249-
// fudge scope.
250-
formal_input_tys_ns = formal_input_tys
251-
.iter()
252-
.map(|&ty| {
253-
// If we replace a (unresolved) inference var with a new inference
254-
// var, it will be eventually resolved to itself and this will
255-
// weaken type inferences as the new inference var will be fudged
256-
// out and lose all relationships with other vars while the former
257-
// will not be fudged.
258-
if ty.is_ty_var() {
259-
return ty;
260-
}
261-
262-
let generalized_ty = self.next_ty_var(call_span);
263-
ocx.eq(&origin, self.param_env, ty, generalized_ty).unwrap();
264-
generalized_ty
265-
})
266-
.collect_vec();
267-
268-
formal_input_tys_ns.as_slice()
269-
} else {
270-
formal_input_tys
271-
};
264+
// Check the well-formedness of expected input tys, as using ill-formed
265+
// expectation may cause type inference errors, see #150316.
266+
for &ty in formal_input_tys {
267+
ocx.register_obligation(traits::Obligation::new(
268+
self.tcx,
269+
self.misc(call_span),
270+
self.param_env,
271+
ty::ClauseKind::WellFormed(ty.into()),
272+
));
273+
}
272274

273275
if !ocx.try_evaluate_obligations().is_empty() {
274276
return Err(TypeError::Mismatch);
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error[E0277]: the size for values of type `[{integer}]` cannot be known at compilation time
2+
--> $DIR/expectated-input-not-satisfying-fn-bounds-issue-149881.rs:15:24
3+
|
4+
LL | <[_]>::into_vec(id(Box::new([0, 1, 2])));
5+
| -- ^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
6+
| |
7+
| required by a bound introduced by this call
8+
|
9+
= help: the trait `Sized` is not implemented for `[{integer}]`
10+
note: required by an implicit `Sized` bound in `id`
11+
--> $DIR/expectated-input-not-satisfying-fn-bounds-issue-149881.rs:10:7
12+
|
13+
LL | fn id<T>(x: Box<T>) -> Box<T> {
14+
| ^ required by the implicit `Sized` requirement on this type parameter in `id`
15+
help: consider relaxing the implicit `Sized` restriction
16+
|
17+
LL | fn id<T: ?Sized>(x: Box<T>) -> Box<T> {
18+
| ++++++++
19+
20+
error: aborting due to 1 previous error
21+
22+
For more information about this error, try `rustc --explain E0277`.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error[E0277]: the size for values of type `[{integer}]` cannot be known at compilation time
2+
--> $DIR/expectated-input-not-satisfying-fn-bounds-issue-149881.rs:15:24
3+
|
4+
LL | <[_]>::into_vec(id(Box::new([0, 1, 2])));
5+
| -- ^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
6+
| |
7+
| required by a bound introduced by this call
8+
|
9+
= help: the trait `Sized` is not implemented for `[{integer}]`
10+
note: required by an implicit `Sized` bound in `id`
11+
--> $DIR/expectated-input-not-satisfying-fn-bounds-issue-149881.rs:10:7
12+
|
13+
LL | fn id<T>(x: Box<T>) -> Box<T> {
14+
| ^ required by the implicit `Sized` requirement on this type parameter in `id`
15+
help: consider relaxing the implicit `Sized` restriction
16+
|
17+
LL | fn id<T: ?Sized>(x: Box<T>) -> Box<T> {
18+
| ++++++++
19+
20+
error: aborting due to 1 previous error
21+
22+
For more information about this error, try `rustc --explain E0277`.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@ revisions: current next
2+
//@ ignore-compare-mode-next-solver (explicit revisions)
3+
//@[next] compile-flags: -Znext-solver
4+
5+
// FIXME(#149379): This should pass, but fails due to fudged expactation
6+
// types which are potentially not well-formed or for whom the function
7+
// where-bounds don't actually hold. And this results in weird bugs when
8+
// later treating these expectations as if they were actually correct..
9+
10+
fn id<T>(x: Box<T>) -> Box<T> {
11+
x
12+
}
13+
14+
fn main() {
15+
<[_]>::into_vec(id(Box::new([0, 1, 2])));
16+
//~^ ERROR: the size for values of type `[{integer}]` cannot be known at compilation time
17+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error[E0277]: `dyn Trait + Send` cannot be unpinned
2+
--> $DIR/expectated-input-not-satisfying-fn-bounds-issue-89299.rs:20:27
3+
|
4+
LL | let _x = Foo(Pin::new(&mut a));
5+
| -------- ^^^^^^ the trait `Unpin` is not implemented for `dyn Trait + Send`
6+
| |
7+
| required by a bound introduced by this call
8+
|
9+
= note: consider using the `pin!` macro
10+
consider using `Box::pin` if you need to access the pinned value outside of the current scope
11+
note: required by a bound in `Pin::<Ptr>::new`
12+
--> $SRC_DIR/core/src/pin.rs:LL:COL
13+
14+
error: aborting due to 1 previous error
15+
16+
For more information about this error, try `rustc --explain E0277`.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error[E0277]: `dyn Trait + Send` cannot be unpinned
2+
--> $DIR/expectated-input-not-satisfying-fn-bounds-issue-89299.rs:20:27
3+
|
4+
LL | let _x = Foo(Pin::new(&mut a));
5+
| -------- ^^^^^^ the trait `Unpin` is not implemented for `dyn Trait + Send`
6+
| |
7+
| required by a bound introduced by this call
8+
|
9+
= note: consider using the `pin!` macro
10+
consider using `Box::pin` if you need to access the pinned value outside of the current scope
11+
note: required by a bound in `Pin::<Ptr>::new`
12+
--> $SRC_DIR/core/src/pin.rs:LL:COL
13+
14+
error: aborting due to 1 previous error
15+
16+
For more information about this error, try `rustc --explain E0277`.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//@ revisions: current next
2+
//@ ignore-compare-mode-next-solver (explicit revisions)
3+
//@[next] compile-flags: -Znext-solver
4+
5+
// FIXME(#149379): This should pass, but fails due to fudged expactation
6+
// types which are potentially not well-formed or for whom the function
7+
// where-bounds don't actually hold. And this results in weird bugs when
8+
// later treating these expectations as if they were actually correct..
9+
10+
use std::pin::Pin;
11+
12+
trait Trait {}
13+
14+
impl Trait for i32 {}
15+
16+
struct Foo<'a>(Pin<&'a mut (dyn Trait + Send)>);
17+
18+
fn main() {
19+
let mut a = 1;
20+
let _x = Foo(Pin::new(&mut a));
21+
//~^ ERROR: `dyn Trait + Send` cannot be unpinned
22+
}

tests/ui/coercion/fudge-inference/fn-ret-trait-object-propagated-to-inputs-issue-149379-1.current.stderr

Lines changed: 0 additions & 32 deletions
This file was deleted.

tests/ui/coercion/fudge-inference/fn-ret-trait-object-propagated-to-inputs-issue-149379-1.next.stderr

Lines changed: 0 additions & 32 deletions
This file was deleted.
Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
//@ revisions: current next
22
//@ ignore-compare-mode-next-solver (explicit revisions)
33
//@[next] compile-flags: -Znext-solver
4+
//@ check-pass
45

5-
// FIXME(#149379): This should pass, but fails due to fudged expactation
6-
// types which are potentially not well-formed or for whom the function
7-
// where-bounds don't actually hold. And this results in weird bugs when
8-
// later treating these expectations as if they were actually correct..
6+
// A regression test for https://github.com/rust-lang/rust/issues/149379.
97

108
fn foo<T>(x: (T, ())) -> Box<T> {
119
Box::new(x.0)
@@ -14,6 +12,4 @@ fn foo<T>(x: (T, ())) -> Box<T> {
1412
fn main() {
1513
// Uses expectation as its struct tail is sized, resulting in `(dyn Send, ())`
1614
let _: Box<dyn Send> = foo(((), ()));
17-
//~^ ERROR mismatched types
18-
//~| ERROR the size for values of type `dyn Send` cannot be known at compilation time
1915
}

0 commit comments

Comments
 (0)