Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

higher-ranked goals in trait goal candidate selection #120

Open
lcnr opened this issue Jul 10, 2024 · 1 comment
Open

higher-ranked goals in trait goal candidate selection #120

lcnr opened this issue Jul 10, 2024 · 1 comment

Comments

@lcnr
Copy link
Contributor

lcnr commented Jul 10, 2024

This compiles with the old solver but currently fails with next:

trait Leak<'a> {}
impl Leak<'_> for Box<u32> {}
impl Leak<'static> for Box<u16> {}

fn impls_leak<T: for<'a> Leak<'a>>() {}
fn direct() {
    // The `Box<u16>` impls fails the leak check,
    // meaning that we apply the `Box<u32>` impl.
    impls_leak::<Box<_>>();
    //[next]~^ ERROR type annotations needed
}

cc rust-lang/rust#119820 which got reverted in rust-lang/rust#127568

We originally intended to never consider the placeholders from higher ranked goals while computing their candidates. However, this resulted in some undesirable breakage and we'll instead have to support this with the next solver before stabilization.

I currently hope to support this by adding OR region bounds and removing ParamEnv preference by returning guidance instead, discussed in https://hackmd.io/-IMJ8e0iTBqvL6X1XuiRdw#Leak-check

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 10, 2024
instantiate higher ranked goals in candidate selection again

This reverts rust-lang#119820 as that PR has a significant impact and breaks code which *feels like it should work*. The impact ended up being larger than we expected during the FCP and we've ended up with some ideas for how we can work around this issue in the next solver. This has been discussed in the previous high bandwidth t-types meeting: https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2024-07-09.20high.20bandwidth.20meeting.

We'll therefore keep this inconsistency between the two solvers for now and will have to deal with it before stabilizating the use of the new solver outside of coherence: rust-lang/trait-system-refactor-initiative#120.

fixes rust-lang#125194 after a beta-backport.

The pattern which is more widely used than expected and feels like it should work, especially without deep knowledge of the type system is
```rust
trait Trait<'a> {}
impl<'a, T> Trait<'a> for T {}

fn trait_bound<T: for<'a> Trait<'a>>() {}

// A function with a where-bound which is more restrictive than the impl.
fn function1<T: Trait<'static>>() {
    // stable: ok
    // with rust-lang#119820: error as we prefer the where-bound over the impl
    // with this PR: back to ok
    trait_bound::<T>();
}

```

r? `@rust-lang/types`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 10, 2024
Rollup merge of rust-lang#127568 - lcnr:undo-leakcheck, r=oli-obk

instantiate higher ranked goals in candidate selection again

This reverts rust-lang#119820 as that PR has a significant impact and breaks code which *feels like it should work*. The impact ended up being larger than we expected during the FCP and we've ended up with some ideas for how we can work around this issue in the next solver. This has been discussed in the previous high bandwidth t-types meeting: https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2024-07-09.20high.20bandwidth.20meeting.

We'll therefore keep this inconsistency between the two solvers for now and will have to deal with it before stabilizating the use of the new solver outside of coherence: rust-lang/trait-system-refactor-initiative#120.

fixes rust-lang#125194 after a beta-backport.

The pattern which is more widely used than expected and feels like it should work, especially without deep knowledge of the type system is
```rust
trait Trait<'a> {}
impl<'a, T> Trait<'a> for T {}

fn trait_bound<T: for<'a> Trait<'a>>() {}

// A function with a where-bound which is more restrictive than the impl.
fn function1<T: Trait<'static>>() {
    // stable: ok
    // with rust-lang#119820: error as we prefer the where-bound over the impl
    // with this PR: back to ok
    trait_bound::<T>();
}

```

r? `@rust-lang/types`
@lcnr
Copy link
Contributor Author

lcnr commented Jan 21, 2025

affected ui tests

  • tests/ui/higher-ranked/leak-check/candidate-from-env-universe-err-2.rs
  • tests/ui/higher-ranked/leak-check/candidate-from-env-universe-err-1.rs
  • tests/ui/higher-ranked/leak-check/leak-check-in-selection-2.rs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: unknown
Development

No branches or pull requests

1 participant