Lazy type-alias-impl-trait take two

[oli-obk]

user visible change 1: RPIT inference from recursive call sites

Lazy TAIT has an insta-stable change. The following snippet now compiles, because opaque types can now have their hidden type set from wherever the opaque type is mentioned.

fn bar(b: bool) -> impl std::fmt::Debug {
    if b {
        return 42
    }
    let x: u32 = bar(false); // this errors on stable
    99
}

The return type of bar stays opaque, you can't do bar(false) + 42, you need to actually mention the hidden type.

user visible change 2: divergence between RPIT and TAIT in return statements

Note that return statements and the trailing return expression are special with RPIT (but not TAIT). So

#![feature(type_alias_impl_trait)]
type Foo = impl std::fmt::Debug;

fn foo(b: bool) -> Foo {
    if b {
        return vec![42];
    }
    std::iter::empty().collect() //~ ERROR `Foo` cannot be built from an iterator
}

fn bar(b: bool) -> impl std::fmt::Debug {
    if b {
        return vec![42]
    }
    std::iter::empty().collect() // Works, magic (accidentally stabilized, not intended)
}

But when we are working with the return value of a recursive call, the behavior of RPIT and TAIT is the same:

type Foo = impl std::fmt::Debug;

fn foo(b: bool) -> Foo {
    if b {
        return vec![];
    }
    let mut x = foo(false);
    x = std::iter::empty().collect(); //~ ERROR `Foo` cannot be built from an iterator
    vec![]
}

fn bar(b: bool) -> impl std::fmt::Debug {
    if b {
        return vec![];
    }
    let mut x = bar(false);
    x = std::iter::empty().collect(); //~ ERROR `impl Debug` cannot be built from an iterator
    vec![]
}

user visible change 3: TAIT does not merge types across branches

In contrast to RPIT, TAIT does not merge types across branches, so the following does not compile.

type Foo = impl std::fmt::Debug;

fn foo(b: bool) -> Foo {
    if b {
        vec![42_i32]
    } else {
        std::iter::empty().collect()
        //~^ ERROR `Foo` cannot be built from an iterator over elements of type `_`
    }
}

It is easy to support, but we should make an explicit decision to include the additional complexity in the implementation (it's not much, see a721052457cf513487fb4266e3ade65c29b272d2 which needs to be reverted to enable this).

PR formalities

previous attempt: #92007

This PR also includes #92306 and #93783, as they were reverted along with #92007 in #93893

fixes #93411
fixes #88236
fixes #89312
fixes #87340
fixes #86800
fixes #86719
fixes #84073
fixes #83919
fixes #82139
fixes #77987
fixes #74282
fixes #67830
fixes #62742
fixes #54895

[matthiaskrgr]

Could we also have a test for #93788 ? :3 (I couldn't find anything)

edit: efcd969 thx!

[jackh726]

It might be obvious, but we should do a crater run before landing this. And also probably wait until after this upcoming release (so we don't land this at the end of a cycle).

[oli-obk]

@bors try @rust-timer queue

It might be obvious, but we should do a crater run before landing this. And also probably wait until after this upcoming release (so we don't land this at the end of a cycle).

yes, definitely. Gonna start it once the try run is complete, and we get perf on the same commit, too

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit efcd9694a93286567ebb50f7ec0bca24ffa0e39b with merge e251c42c969d6f8dbaae00e1028489c3a29f8789...

[oli-obk]

This never worked for fn foo() -> (impl Debug, i32), but was only used for diagnostics, which have since been fixed to work without this field.

[oli-obk]

Sadly (due to #93794 working on stable) I had to re-introduce inference variables for the return type (at least for RPIT). This makes

fn foo(b: bool) -> impl IntoIterator<Item = ()> {
    if b {
        return vec![()];
    }
    [()].into_iter().collect()
}

pass, which was probably unintentionally allowed (there were no tests for it).

The equivalent for TAIT does not work, and I don't think we should support it, because TAITs can show up everywhere, not just in the return type.

[Aaron1011]

Are we planning to support this permanently, or will this be removed after a warning period?

[oli-obk]

I don't have a strong opinion here. It's annoying to support in the compiler, but also used in the ecosystem, so 🤷

[bors]

☀️ Try build successful - checks-actions
Build commit: e251c42c969d6f8dbaae00e1028489c3a29f8789 (e251c42c969d6f8dbaae00e1028489c3a29f8789)

[rust-timer]

Queued e251c42c969d6f8dbaae00e1028489c3a29f8789 with parent 30b3f35, future comparison URL.

[oli-obk]

@craterbot check

[craterbot]

👌 Experiment pr-94081 created and queued.
🤖 Automatically detected try build e251c42c969d6f8dbaae00e1028489c3a29f8789
🔍 You can check out the queue and this experiment's details.

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

[rust-timer]

Finished benchmarking commit (e251c42c969d6f8dbaae00e1028489c3a29f8789): comparison url.

Summary: This benchmark run shows 10 relevant improvements 🎉 but 39 relevant regressions 😿 to instruction counts.

• Average relevant regression: 0.9%
• Average relevant improvement: -0.5%
• Largest improvement in instruction counts: -0.6% on incr-full builds of unused-warnings check
• Largest regression in instruction counts: 3.3% on full builds of deeply-nested-async check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led 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-review -S-waiting-on-perf +perf-regression

[craterbot]

🚧 Experiment pr-94081 is now running

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

[plazmoid]

By the way, could this PR unlock easier and more convenient ways to reimplement impl-traits-in-bindings?

[oli-obk]

Yes! That is in fact one of the motivating use cases of lazy TAIT. Impl trait in bindings becomes easy and trivially correct (it's just a desugaring to a TAIT after all)

[g2p]

How does Crater take so long "Generating report"? The run itself was completed half a day ago. 🌄

[Mark-Simulacrum]

@craterbot retry-report

[craterbot]

🛠️ Generation of the report for pr-94081 queued again.

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

[craterbot]

🎉 Experiment pr-94081 is completed!
📊 9 regressed and 3 fixed (212989 total)
📰 Open the full report.

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