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

Inference failure with type_changing_struct_update #101970

Open
Tracked by #86555
crlf0710 opened this issue Sep 18, 2022 · 13 comments
Open
Tracked by #86555

Inference failure with type_changing_struct_update #101970

crlf0710 opened this issue Sep 18, 2022 · 13 comments
Labels
C-bug Category: This is a bug. F-type-changing-struct-update `#![feature(type_changing_struct_update)]`

Comments

@crlf0710
Copy link
Member

crlf0710 commented Sep 18, 2022

In the crater run #98456, fajita crate regression exhibited inference failure. Here's a minimal repro:
// when feature gate is enabled below, the compilation fails.

#![feature(type_changing_struct_update)]

#[derive(Default)]
pub struct Foo<P, T> {
    pub t: T,
    pub v: P
}

impl<P: Default, T:Default> Foo<P, T> {
    pub fn o(t: T) -> Self {
        Foo { t, ..Default::default() }
    }
}
@crlf0710 crlf0710 added C-bug Category: This is a bug. F-type-changing-struct-update `#![feature(type_changing_struct_update)]` labels Sep 18, 2022
@crlf0710
Copy link
Member Author

Just creating an issue for easier tracking.

cc #86555

@nikomatsakis
Copy link
Contributor

We certainly knew this was a possibility when authoring the RFC -- based on the Zulip conversation, I believe that there were a number of inference failures of this kind, right?

The options on the table that I see are:

  • Make the change anyway, breaking the crates.
  • Make the change over an edition.
  • Give up.

My sense was that there was a significant amount of breakage, making the first not an option. In that case, if we want to proceed, we should probably do the work to plan how we can make this an edition change (in particular, can we do migration?). I think we should be able to.

I'm going to nominate this issue to make the @rust-lang/lang team aware, but I'd appreciate it @crlf0710 if you or perhaps @compiler-errors could drop in more details about the scale of breakage found on crater.

@nikomatsakis

This comment was marked as outdated.

@nikomatsakis
Copy link
Contributor

@rustbot label +I-lang-nominated

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Sep 18, 2022
@WaffleLapkin
Copy link
Member

For some context, there is a zulip thread about finishing work on type-changing struct update.

And here is a MRE of this particular problem:

#![feature(type_changing_struct_update)]

#[derive(Default)]
struct Struct<T>(T);

fn _f() { Struct { 0: (), ..Default::default() }; }

If I understood correctly, @compiler-errors has an idea how we can hack inference to allow such examples. It's a hack though, so we may prefer to just do an editional breakage...

@compiler-errors
Copy link
Member

For context, here's the experimentation I have done:

But even with those inference hacks there are still cases where we get failures.

The inference hack is in a6e4db2 but it's very much a hack.

@SparrowLii
Copy link
Member

SparrowLii commented Sep 20, 2022

I understood the cause of the problem to some extent and should be able to submit a PR soon to fix it

@SparrowLii
Copy link
Member

@rustbot claim

@scottmcm
Copy link
Member

scottmcm commented Oct 6, 2022

We discussed this in the lang meeting this week.

I don't think we had agreement on what the way forward should be, but we had consensus that

  • The inference breakage of just making the basic change is more than we're willing to accept.
  • We're interested in seeing a plan to make a migration path reasonable over an edition
    • We don't know what that plan should be.
    • But needing ..<Foo<A, B>>::default() isn't it.

@rustbot label -I-lang-nominated

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Oct 6, 2022
@SparrowLii SparrowLii removed their assignment Oct 10, 2022
@SparrowLii
Copy link
Member

SparrowLii commented Oct 10, 2022

I came up with my solution, but it looks like the language team wants a different approach. So unassigned myself.

@Crazytieguy
Copy link

Are there plans for trying to get this in to the 2024 edition?

@compiler-errors
Copy link
Member

I don't think so. The inference fallout is really dramatic without hacks, and (speaking as an individual contributor, and not for the whole types team) I don't believe there is really a good, principled hack to implement here that would make the types team happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-type-changing-struct-update `#![feature(type_changing_struct_update)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants