instantiate higher ranked goals outside of candidate selection, remove the `coherence_leak_check` future compat lint

[lcnr]

• #59490
• #56105

---

and adapt the old solver to not rely on universe errors for higher ranked goals to impact candidate selection. This matches the behavior of the new solver: https://github.com/rust-lang/trait-system-refactor-initiative/issues/34. See https://hackmd.io/gstwC83ORaG7V29w54nMdA for more details about this change

r? @nikomatsakis

[rustbot]

Type relation code was changed

cc @compiler-errors, @lcnr

[lcnr]

@bors try @rust-timer queue

(perf and need crater for https://github.com/rust-lang/trait-system-refactor-initiative/issues/34)

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

:hourglass: Trying commit e49e69b68f8d8c4c7a01355e54a5593c86a1b5bf with merge b69bfeecc49bf5661cf54497638430035ff61aa4...

[bors]

:sunny: Try build successful - checks-actions Build commit: b69bfeecc49bf5661cf54497638430035ff61aa4 (b69bfeecc49bf5661cf54497638430035ff61aa4)

[rust-timer]

Queued b69bfeecc49bf5661cf54497638430035ff61aa4 with parent e9271846294c4ee5bd7706df68180320c0b5ff20, future comparison URL. There are currently 0 preceding artifacts in the queue. It will probably take at least ~1.1 hours until the benchmark run finishes.

[rust-timer]

Finished benchmarking commit (b69bfeecc49bf5661cf54497638430035ff61aa4): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never @rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.2%, 0.8%]	8
Regressions ❌ (secondary)	1.2%	[1.2%, 1.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.5%	[0.2%, 0.8%]	8

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.0%	[0.6%, 1.2%]	3
Regressions ❌ (secondary)	1.7%	[1.3%, 2.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.9%	[-1.9%, -1.9%]	1
All ❌✅ (primary)	1.0%	[0.6%, 1.2%]	3

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	6.1%	[6.1%, 6.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 666.26s -> 666.778s (0.08%) Artifact size: 308.39 MiB -> 308.42 MiB (0.01%)

[lcnr]

@craterbot check

[craterbot]

:ok_hand: Experiment pr-119820 created and queued. :robot: Automatically detected try build b69bfeecc49bf5661cf54497638430035ff61aa4 :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:construction: Experiment pr-119820 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:tada: Experiment pr-119820 is completed! :bar_chart: 61 regressed and 5 fixed (406907 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[lcnr]

looking at https://crates.io/crates/rene, I think the error pattern is the following:

trait Trait {}

impl<T: Trait> Trait for &T {}
impl Trait for u32 {}


fn hr_bound<T>() 
where
    for<'a> &'a T: Trait,
{}

fn foo<T>()
where
    T: Trait,
    for<'a> &'a &'a T: Trait,
{
    hr_bound::<&T>();
    // We get a universe error when using the `param_env` candidate
    // but are able to successfully use the impl candidate. Without
    // the leak check both candidates may apply and we prefer the
    // `param_env` candidate in winnowing.
}

fn main() {}

[lcnr]

@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-119820/retry-regressed-list.txt

[craterbot]

:ok_hand: Experiment pr-119820-1 created and queued. :robot: Automatically detected try build b69bfeecc49bf5661cf54497638430035ff61aa4 :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:construction: Experiment pr-119820-1 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:tada: Experiment pr-119820-1 is completed! :bar_chart: 23 regressed and 0 fixed (61 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[lcnr]

another, simpler pattern broken by this change

trait Trait<'a> {}

trait OtherTrait {}
impl<'a, T: OtherTrait> Trait<'a> for T {}

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

fn not_hr<'b, T: Trait<'b> + OtherTrait>() {
    impl_hr::<T>();
} 

[lcnr]

and the derive examples are something like this:

use serde::{de::DeserializeOwned, Deserialize, Deserializer};

struct NeedsOwned(bool);
impl<'de> Deserialize<'de> for NeedsOwned
where
    bool: DeserializeOwned,
{
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        Ok(NeedsOwned(<bool as Deserialize>::deserialize(deserializer)?))
    }
}

pub fn deserialize<'de, D>(deserializer: D) -> Result<bool, D::Error>
where
    bool: Deserialize<'de>,
    D: Deserializer<'de>,
{
    // requires `bool: DeserializeOwned` which requires `for<'de> bool: Deserialize<'de>`
    match <NeedsOwned as ::serde::Deserialize>::deserialize(deserializer) {
        Ok(value) => Ok(value.0),
        Err(e) => Err(e),
    }
}

[Jules-Bertholet]

I'm concerned that this could affect efforts to add variance to trait implementations. For example, if Sub <: Super and an impl PartialEq<Foo> for Super { /* ... */ } exists, the compiler could theoretically generate an impl of PartialEq<Foo> for Sub on its own. Currently, it's impossible to write an impl that would conflict with such a feature without triggering the coherence_leak_check lint. (With pattern types based on subtyping, the ability to generate these impls could become much more important)

[lcnr]

another minimization

trait Trait<T> {}
impl<T> Trait<T> for T {}
impl<T> Trait<T> for &T {}

fn get<S>(_: &S)
where
    for<'a> &'a u32: Trait<S>,
{}

fn render(template: &Box<u32>) {
    get(template);
    // `for<'a> &'a u32: Trait<?S>` only has one applicable candidate,
    // so `?S` gets eagerly inferred to `u32`, causing coercion to add an
    // auto-deref step here.
}

fn main() {}

[lcnr]

@bors try

[bors]

:hourglass: Trying commit cab2f4efbd782de8e7856d0b2ccaac70c8538604 with merge eb56c88b636b00cb5c6f55268c3020b5eacc9009...

[bors]

:boom: Test timed out

[lcnr]

@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-119820-1/retry-regressed-list.txt

[craterbot]

:ok_hand: Experiment pr-119820-2 created and queued. :robot: Automatically detected try build b69bfeecc49bf5661cf54497638430035ff61aa4 :warning: Try build based on commit e49e69b68f8d8c4c7a01355e54a5593c86a1b5bf, but latest commit is cab2f4efbd782de8e7856d0b2ccaac70c8538604. Did you forget to make a new try build? :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[lcnr]

@craterbot abort

[craterbot]

:wastebasket: Experiment pr-119820-2 deleted!

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[lcnr]

@bors try

[bors]

:hourglass: Trying commit cab2f4efbd782de8e7856d0b2ccaac70c8538604 with merge d085246e9641389dbd198782e7b0fca4bce73196...

[bors]

:boom: Test timed out