implied bounds: normalize in the proper param_env #105982
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred in engine.rs, potentially modifying the public API of cc @lcnr |
|
☔ The latest upstream changes (presumably #106103) made this pull request unmergeable. Please resolve the merge conflicts. |
| // FIXME(#84533): We probably shouldn't use output types in implied bounds. | ||
| // This would reject this fn `fn f<'a, 'b>() -> &'a &'b () { .. }`. |
There was a problem hiding this comment.
I don't think this should be how we fix #84533 because you'd still have an builtin impl with an associated type which isn't well formed.
The correct fix is to actually check that the return type is well formed even without calling the function. This needs implications for binders though.
Until then something like #106807 might work as a stopgap (again having the issue of unnormalized projections though)
| let infcx = tcx.infer_ctxt().build(); | ||
| let normalized = infcx | ||
| .at(&ObligationCause::dummy(), param_env) | ||
| .query_normalize(value) |
There was a problem hiding this comment.
query_normalize eagerly evaluates constants causing cycle errors if there are unevaluated constants in the fn sig.
During analysis normalize should be used instead.
|
Switching to waiting on author for a few changes, then it looks we're ok here? @rustbot author |
rustbot
added
S-waiting-on-author
|
@aliemjay Ping from triage: Can you post your status on this PR? This has sat idle for a few months. |
use implied bounds when checking opaque types
During opaque type inference, we check for the well-formedness of the hidden type in the opaque type's own environment, not the one of the defining site, which are different in the case of TAIT.
However in the case of associated-type-impl-trait, we don't use implied bounds from the impl header. This caused us to reject the following:
```rust
trait Service<Req> {
type Output;
fn call(req: Req) -> Self::Output;
}
impl<'a, Req> Service<&'a Req> for u8 {
type Output= impl Sized; // we can't prove WF of hidden type `WF(&'a Req)` although it's implied by the impl
//~^ ERROR type parameter Req doesn't live long enough
fn call(req: &'a Req) -> Self::Output {
req
}
}
```
although adding an explicit bound would make it pass:
```diff
- impl<'a, Req> Service<&'a Req> for u8 {
+ impl<'a, Req> Service<&'a Req> for u8 where Req: 'a, {
```
I believe it should pass as we already allow the concrete type to be used:
```diff
impl<'a, Req> Service<&'a Req> for u8 {
- type Output= impl Sized;
+ type Output= &'a Req;
```
Fixes rust-lang#95922
Builds on rust-lang#105982
cc `@lcnr` (because implied bounds)
r? `@oli-obk`
use implied bounds when checking opaque types
During opaque type inference, we check for the well-formedness of the hidden type in the opaque type's own environment, not the one of the defining site, which are different in the case of TAIT.
However in the case of associated-type-impl-trait, we don't use implied bounds from the impl header. This caused us to reject the following:
```rust
trait Service<Req> {
type Output;
fn call(req: Req) -> Self::Output;
}
impl<'a, Req> Service<&'a Req> for u8 {
type Output= impl Sized; // we can't prove WF of hidden type `WF(&'a Req)` although it's implied by the impl
//~^ ERROR type parameter Req doesn't live long enough
fn call(req: &'a Req) -> Self::Output {
req
}
}
```
although adding an explicit bound would make it pass:
```diff
- impl<'a, Req> Service<&'a Req> for u8 {
+ impl<'a, Req> Service<&'a Req> for u8 where Req: 'a, {
```
I believe it should pass as we already allow the concrete type to be used:
```diff
impl<'a, Req> Service<&'a Req> for u8 {
- type Output= impl Sized;
+ type Output= &'a Req;
```
Fixes rust-lang#95922
Builds on rust-lang#105982
cc ``@lcnr`` (because implied bounds)
r? ``@oli-obk``
|
Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks |
TL;DR This moves the normalization of implied wf types from
ObligationCtxt::assumed_wf_typestoTyCtxt::assumed_wf_typesquery and also implements #105779 for the query.Using different ParamEnv's when normalizng wf types can lead to different results and is more error-prone. Now we are only using the original item's param env. I can't think of a way this changes user-facing behavior, only because #98852 isn't fixed yet.
The also uses early and late binders for
assumed_wf_typesquery. The motivation was cleanly fix #95922, which needs to substitute early-bound params for the defined opaque type.r? @lcnr