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

Privacy-respecting Functional Record Update (pr-FRU) #736

Merged
merged 3 commits into from
Feb 10, 2015

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Jan 26, 2015

Change Functional Record Update (FRU) for struct literal expressions to respect struct privacy.

(rendered)

Spawned from rust-lang/rust#21407 (comment)

[edited to link to final rendered version]

@pnkfelix pnkfelix changed the title first draft of privacy-respecting-fru Privacy respecting FRU Jan 26, 2015
@pnkfelix pnkfelix changed the title Privacy respecting FRU Privacy-respecting Functional Record Update (pr-FRU) Jan 26, 2015
@nikomatsakis
Copy link
Contributor

I'm in favor of this RFC. I'd probably phrase it somewhat stronger and say that the semantics can be summarized as FRU being sugar for expanding out the full set of fields. Full stop. (This implies the contents of this RFC, among other things.) Certainly this is how the linear value behavior works now.

@nikomatsakis
Copy link
Contributor

(One clarification: I don't see this as a change to semantics. Rather, I think that the "expand to a bunch of fields" is the current semantics, and there is a bug in the privacy code.)

@pnkfelix
Copy link
Member Author

@nikomatsakis well, i'd say based on the description from rust-lang/rust#21407, it was at least worth an RFC. (i.e., I think there are other (legitimate) mental models on how FRU works).

@pnkfelix
Copy link
Member Author

@nikomatsakis (having said that, I would be happy to reclassify this as a bug rather than an RFC if we don't actually see any comments in the week following its posting.)

@eternaleye
Copy link

One thing I'm not entirely clear on - how does this interact with Default::default()?

To clarify: Default::default() only happens via an impl in the same module as the struct itself, so the data for the private fields is coming from a source that has a right to know them. But if you treat it as mere sugar for expanding out the fields, then the place Default::default() is invoked - which doesn't have that right - gets in the way. So far as I can see, that leaves Default::default()'s usefulness rather severely curtailed... which seems to be a somewhat crippling problem to me.

Personally, there are some things I'm going to be writing (including a few C library bindings) where there are unavoidably going to be quite a few parameters to a constructor. Default::default() with FRU provides a rather nice, readable, and low-effort way of dealing with it, but if Default::default() can't actually initialize the private members of the structure, I'll have to do some rather annoying dancing around with separate structs for public and private parameters, and marshaling the data around into the internal representation.

@nikomatsakis
Copy link
Contributor

@pnkfelix I don't mind the opening of an RFC, and I agree there are other legitimate mental models, but it's just that I think that the "expand to a bunch of fields" is the one that best matches how most of the code operates, which is why I say that is the "current semantics".

For example, the following program:

#![allow(unused_variables)]
#![allow(dead_code)]

struct Noisy(u32);

impl Drop for Noisy {
    fn drop(&mut self) {
        println!("Goodbye {}", self.0);
    }
}

struct Foo {
    x: u32,
    y: Noisy,
}

fn main() {
    let foo = Foo { x: 1, y: Noisy(0) };
    {
        let bar = Foo { y: Noisy(1), ..foo };
        println!("Hi!");
    }
    println!("Ho!");
}

does not drop Noisy(0) at the point of using FRU.

@nikomatsakis
Copy link
Contributor

@eternaleye can you spell out the example you have in mind a bit more?

@pnkfelix
Copy link
Member Author

@nikomatsakis I imagine @eternaleye is talking about this pattern:

struct Foo { pub x: u32, private_1: u32, private_2: String }
impl Default for Foo { ... }
Foo { pub_x: my_chosen_x, ..Default::default() }

However, it seems to me like you can still employ the workaround described in the RFC to deal with this, by writing your code like this:

struct Foo { pub x: u32, pub _hidden: FooPrivate }
struct FooPrivate {  private_1: u32, private_2: String }
impl Default for Foo { ... }
Foo { pub_x: my_chosen_x, ..Default::default() }

(which I guess is what @eternaleye meant in saying "annoying dancing around with separate structs for public and private parameters")

@eternaleye
Copy link

@pnkfelix Yes, that's it exactly. To expand on why I feel it's "annoying dancing around", once you have separate structs for public and private members it rather obviates the need for public/private on the member level at all - you can simply have the private struct remain unexported. It bothers me from the DRY angle, as well as "Why have member-level privacy, if we never use it and just use separate structs?"

Although if FooPrivate is a public member of Foo then it must be a public type, and here we start going down the rabbit hole of 'ick'.

(I like member-level privacy, it's just that if we make it inflexible then it rather undermines the feature)

Also, it getting in the way of Default::default() irritates me on a conceptual level - Default::default() is clearly a trusted source of data (because of the rules on impl), and it rather bothers me that the programmer can't tell a constructor to draw from a source it trusts merely because the programmer isn't trusted.

In a sense, you can see it as a capability model - Default::default() has the "Access private members on Foo" cap, while the programmer has the "Access this specific Foo" cap. When the programmer calls Default::default() in FRU, I feel it should operate more like the programmer passing their capability to Default::default(), which can compose the two capabilities into "Access private members on this specific Foo" - whereas simply expanding it destroys Default::default() as an actor in the model, making the programmer's cap the only one available.

@theemathas
Copy link

-1 for this. I'd prefer functional update to just consume non-Copy values.

In other words, for any type Foo (Copy or non-Copy), the following code snippets would be equivalent:

new_val = Foo { a: x, ..old_val };
new_val = {
    let mut __compiler_internal: Foo = old_val;
    __compiler_internal.a = x;
    __compiler_internal
};

Which is much easier to understand and much easier to maintain in terms of backward-compatibility.

@nikomatsakis
Copy link
Contributor

Given that this RFC seems to be in the spirit of the existing language semantics most closely, we have decided to accept it.

@Centril Centril added A-typesystem Type system related proposals & ideas A-update-syntax Struct update syntax related proposals & ideas A-privacy Privacy related proposals & ideas A-expressions Term language related proposals & ideas labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-expressions Term language related proposals & ideas A-privacy Privacy related proposals & ideas A-typesystem Type system related proposals & ideas A-update-syntax Struct update syntax related proposals & ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants