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

Desugaring of struct update syntax is unnecessarily restrictive for private fields #63538

Open
Osspial opened this issue Aug 13, 2019 · 3 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Osspial
Copy link

Osspial commented Aug 13, 2019

Take the following example struct:

mod module {
    #[derive(Default)]
    pub struct Foo {
        pub a: i32,
        b: u32
    }
}

Let's say you don't have access to b, and you want to create an instance of Foo with a non-default value for a. The idiomatic way to do that is with the struct update syntax, like so:

let foo = Foo {
    a: 2,
    ..Foo::default()
};

Right now, that fails with the following error:

error[E0451]: field `b` of struct `main::module::Foo` is private
  --> src/main.rs:15:7
   |
15 |     ..Foo::default()
   |       ^^^^^^^^^^^^^^ field `b` is private

That error is unintuitive since we never directly reference b in our source code. What's more, it's entirely unnecessary, since it's merely the side effect of what seems to be the current desugaring strategy, as follows:

let foo = {
    // we create temp_a before calling Foo::default() to preserve source code ordering
    let temp_a = 2;
    let temp = Foo::default();
    Foo {
        a: temp_a,
        b: temp.b
    }
};

Ideally we'd desugar to the following, which would allow the initial example to compile as expected:

let foo = {
    let temp_a = 2;
    let mut temp = Foo::default();
    temp.a = temp_a;
    temp
};

This issue proposes moving to the second method. I can't see any disadvantages to doing that, although that may just be because I'm not looking close enough.

This admittedly changes the language's behavior, but I'm submitting it as an issue instead of an RFC because the current behavior seems unintentional and this is an extremely minor change. I'll happily create an RFC if that's necessary, though.

@Osspial Osspial changed the title Desugaring of struct update syntax is unnecessarily restrictive Desugaring of struct update syntax is unnecessarily restrictive for private fields Aug 13, 2019
@jonas-schievink jonas-schievink added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Aug 13, 2019
@petrochenkov
Copy link
Contributor

I actually tried to implement the alternative desugaring, the results can be found in this internals post.

@Osspial
Copy link
Author

Osspial commented Aug 14, 2019

@petrochenkov Those are interesting points. If that's the case, then it seems my guess about how the desugaring currently works isn't correct. What exactly does the struct update syntax desugar to right now?

(for people reading along that don't want to look into the other thread - the current implementation lets you do this:

#[derive(Debug)]
struct Foo {
    pub a: i32,
}

let a = Foo {
    a: 0
};
let b = Foo{..a};

println!("{:?} {:?}", a, b);
}

which my proposed implementation wouldn't allow)

@kvark
Copy link
Contributor

kvark commented Jun 3, 2020

@Osspial we want this in wgpu to have extensible structs without noisy builder patterns.

I understand the example your posted, and I appreciate @petrochenkov findings!

My question is basically, can this be shaped in a way that doesn't change the semantics of ..a initialization?

For example, one of the concerns I see here is that it could allow copying whole structs that otherwise don't have #[derive(Copy)] on them. Can we write down the rules for private fields initialization in a way that takes that into account? I.e.

is it a private field? if not, proceed as usual
is the source container borrowed (as opposed to moved)? if not, move the value in
is the source container copyable? if yes, copy the value in, otherwise:
issue a compile error, specifying that there is a field Xxx in a borrowed struct that can't be initialized because the struct is not Copy.

I believe that would be non-breaking while preserving all the benefits this issue tries to get.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants