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

Allow fields in traits that map to lvalues in impl'ing type #1546

Closed
wants to merge 5 commits into from

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Mar 16, 2016

UPDATE: The finishing touches and final conversation on this RFC are taking place in a dedicated repository, as described in this comment.


The primary change proposed here is to allow fields within traits and then permit access to those fields within generic functions and from trait objects:

  • trait Trait { field: usize }
  • Implementing type can "direct" this field to any lvalue within itself
  • All fields within a trait or its supertraits must be mapped to
    disjoint locations

Fields serve as a better alternative to accessor functions in traits. They are more compatible with Rust's safety checks than accessors, but also more efficient when using trait objects.

Many of the ideas here were originally proposed in #250 in some form. As such, they represent an important "piece of the puzzle" towards solving #349.

cc @eddyb @aturon @rust-lang/lang

Rendered view.

Planned edits

  • Adopt let syntax for declaring fields in traits and impls. (decided against this)
  • Clarify that there are "private type in public API" rules that apply to the type of a field.
  • Decide whether mut is required for mutable access. (At minimum, add as an unresolved question.)
  • Typo

@nikomatsakis nikomatsakis added the T-lang Relevant to the language team, which will review and decide on the RFC. label Mar 16, 2016
@nikomatsakis nikomatsakis self-assigned this Mar 16, 2016
@oli-obk
Copy link
Contributor

oli-obk commented Mar 16, 2016

#1215 also addresses the borrowck problem with accessors, but still has the poor performance issue and there wasn't any talk about traits (yet). The suggestion was some kind of explicit disjoint partitions (e.g. struct field names) that can be mentioned in references to allow partial borrows.


```rust
trait Trait {
field1: Type1, // <-- fields within a block separated by commas.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possibility is to declare "fields" (accessors) using a syntax similar to variable declaration:

trait Trait {
    let field1: Type1;
    let field2: Type2;

    fn foo();
}

This also signifies that it isn't really a field, but more an associated value. It also looks nicer in impls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this syntax is mentioned 10 lines below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, well. I missed that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possibility is to declare "fields" (accessors) using a syntax similar to variable declaration:

trait Trait {
    let field1: Type1;
    let field2: Type2;

    fn foo();
}

This also signifies that it isn't really a field, but more an associated value.

Yeah, I don't hate this. It might be better, all things considered. I
like having all trait items start with a keyword, probably helps us
with future expansions of the syntax, and avoids the awkward , vs
; quesiton.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also has a symmetry to type.

@Ericson2314
Copy link
Contributor

I like this a lot---it's broadly useful even outside the realm of "virtual structs", and very orthogonal to the listed future work, which are the good smells for this sort of thing.

[Kinda off topic] Another route of generalization is "first class lvalues". I don't know what his would look like, but it might be useful wrt things like map entry API. Teaching the borrow checker that entries for disjoint keys are disjoint would be neat. This is absolutely out of scope for this RFC, but if this is accepted, it opens the door to further exploration in that direction---great!

languages. This means that if, e.g., `Circle` wanted to override the
`highlight` method but also call out to the prior version, it can't
easily do so. Super calls are not strictly needed thoug, as one can
always refactor the super call into a free fn.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section contains references to types such as Container and Circle which are not mentioned in the RFC anywhere else. Probably a left-over from previous revisions?

@Manishearth
Copy link
Member

Note that as an inheritance system this is not useful for Servo's DOM. This is just a datapoint, not a reason to block it 😄 Otherwise big 👍 , sounds useful.

@petrochenkov
Copy link
Contributor

Any plans to incorporate mutability in this RFC?
E.g. if I want to emulate a getter function with a trait field, then I wouldn't want this field to be mutable.
Maybe these fields should even be immutable by default.

trait Tr {
    x: u8, // Immutable field access, "getter"
    mut y: u8, // Mutable field access, "setter/getter"
}

@eddyb
Copy link
Member

eddyb commented Mar 17, 2016

@petrochenkov That would be great with fields in inherent impls!
It would mean you can have a private field with an immutable public view in an impl, so you can mutate it, but not anyone else.

@jFransham
Copy link

Big 👍 on this, I personally would prefer that trait fields are not immutable by default, but that mutability is controlled by whether or not the concrete value is mutable. I think if we can work out a general-case disjointness checking solution (the syntax that comes to mind is fn modstigate(self: &Self { field_a }) -> &TypeOfFieldA) then we could keep the semantics of trait fields and struct fields consistent, and use privacy to control field mutability on both (like we do already).

@kylone
Copy link

kylone commented Mar 17, 2016

@jFransham Just to be clear, "use privacy to control field mutability" means using Cell & RefCell in std::cell, right?

@pnkfelix
Copy link
Member

@Manishearth

Note that as an inheritance system this is not useful for Servo's DOM

Just to be clear: would something like the extension described in the Embedded notation and prefix layout section do more to address Servo's use case?

Or do you more broadly mean that we need all the things list in the Other changes section that followed that?

Or do you mean that there is something Servo needs that is not addressed by the items enumerated there?

@Manishearth
Copy link
Member

@pnkfelix embedded notation works pretty well.

@golddranks
Copy link

@jFransham I don't quite understand what you're saying, but traits are basically interfaces. You code against an interface. If you mean by "concrete value" a field in the struct that implements a trait, how could you code generically against the interface, without the trait saying whether the value is mutable or immutable? You can't.

@eddyb
Copy link
Member

eddyb commented Mar 17, 2016

@golddranks The mutability depends on whether the value implementing the trait is in a mutable slot or not. This is already the case, e.g. if you write accessors, you can call setters that take &mut self only if the value is in a mutable slot.

@golddranks
Copy link

@eddyb Ah, I see. Pardon my ignorance.

@nikomatsakis
Copy link
Contributor Author

@petrochenkov

Any plans to incorporate mutability in this RFC?

I did not have any such plans. It's an interesting thought. I sort of wish we declared fields as mut as well, in which case this would be consistent. But we don't.

I think I would be more in favor if we also planned to add a mut keyword to ordinary fields and then to lint against assigning directly to a field (or a projection from a field) unless it is declared as mut. Put another way: you would be able to assign to x.f if (a) x is mutable, as today and (b) f is mutable. If (a) is violated, you get an error (as today). If (b) is violated, the code compiles, but you get a lint warning. Obviously the rules would still be mildly different then but if you followed "best practices" it would be consistent.

@nikomatsakis
Copy link
Contributor Author

@Manishearth

Note that as an inheritance system this is not useful for Servo's DOM. This

I think you and @pnkfelix hashed this out, but to be clear: as the RFC states, this RFC alone is not intended to solve Servo's "DOM problem", but it is a major building block towards doing so. The section on future work lays out the additional steps that I believe would be needed to make a truly ergonomic DOM implementation. If you think anything else is required, it'd be good to speak up. =)

@Manishearth
Copy link
Member

Yeah, understood. I personally don't really feel Servo needs a better DOM solution at this stage (If it exists, sure, we'd use it, but I don't want to push for it). I'm happy with our current set of hacks, and the only (minor) improvement I'd like would be some layout-guarantees so that the transmutes aren't technically UB.

I was just pointing out that if the motivation behind this was partially due to use cases like Servo's DOM, it doesn't apply cleanly there.

Basically, Servo would need cheap upcasting. I think with this proposal you need to use trait objects to get that effect, if it is even possible.

@nikomatsakis
Copy link
Contributor Author

I don't have time to edit the draft now, but I've added a "planned edits" section to the main area.

@kbknapp
Copy link

kbknapp commented Mar 18, 2016

Huge 👍 I've wanted this for a while but there's no way I could have articulated it that well!

@omarabid
Copy link

Another bump for this feature. It seems that it has stalled.

@garma83
Copy link

garma83 commented Jan 9, 2022

Another bump. Would be very useful. Main reason would be to get rid of large amounts of getter methods.

@ic3man5
Copy link

ic3man5 commented Apr 10, 2022

Another bump. +1 from me.

I found this from here:

https://stackoverflow.com/a/48470287/926217

@dbsxdbsx
Copy link

Another bump. +1 from me.

I found this from here:

https://stackoverflow.com/a/48470287/926217

But if this feature is done, does it mean that it is a kind of inheritance mechanism? Seems that Rust team don't like inheritance. And for most situation, composition is better than inheritance.

@golddranks
Copy link

golddranks commented Apr 11, 2022

@dbsxdbsx Rust has nothing ideological against inheritance, features are discussed by their merits and costs. But, this feature would not behave the same way as inheritance. (Single) inheritance defines a tree-like structure of canonical relationships between types where those below in the hierarchy always conform to the "interface" defined by those above. However, traits behave like interfaces but they are NOT types, so in Rust, the hierarchy of types is always flat. This feature doesn't change that. This feature is already currently implementable by setters and getters, but the difference is that native support for field mappings would solve some pain points around the borrow checker, as the compiler is then able to reason about the disjointness of the fields.

@dbsxdbsx
Copy link

dbsxdbsx commented Apr 11, 2022

@dbsxdbsx Rust has nothing ideological against inheritance, features are discussed by their merits and costs. But, this feature would not behave the same way as inheritance. (Single) inheritance defines a tree-like structure of canonical relationships between types where those below in the hierarchy always conform to the "interface" defined by those above. However, traits behave like interfaces but they are NOT types, so in Rust, the hierarchy of types is always flat. This feature doesn't change that. This feature is already currently implementable by setters and getters, but the difference is that native support for field mappings would solve some pain points around the borrow checker, as the compiler is then able to reason about the disjointness of the fields.

@golddranks, IMHO, I don't see the word flat means you refer to . From my coding experience, like c++, c# and python (has one or multiple layer hierarchy mechanism), if a field can be defined in trait, it seems to be no difference to hierarchy mechanism in c++, since trait can now also be inherited by another trait in rust. But I do agree that the trait system/concept is more powerful in rust, as it makes generic programming more pointed like using concept in modern c++.

By the way, do you agree on " Composition is always better than inheritance"?

@golddranks
Copy link

So the difference is in that trait is not usable by itself, it is always "abstract". Unlike with languages with inheritance subtyping you also can't have a value or a variable of the "base type/trait". This is what I mean by flat: there are no subtyping relationships between the trait and the implementing type. The trait would also define only the existence of the fields, but no memory layout for them, so all the implementing types would be free to store the fields in any way they wish, unlike with inheritance, where by the usual implementation, the memory layout of the child types are required to contain the memory layout of the base type as a prefix.

I don't agree on "composition is always better than inheritance". I'd say, "composition is usually better than inheritance", but that discussion is kinda out of scope.

@kevinringier
Copy link

One more bump for this feature :)

Also found this from https://stackoverflow.com/a/48470287/926217

@tochiu
Copy link

tochiu commented Aug 18, 2022

Bumping for this too, just came across this while learning the lang :)

@Rigidity
Copy link

This would greatly improve code readability. Would love to see this implemented. Using getter methods just feels like a hacky workaround for something which should (in my opinion be possible directly.

The main use case I can think of is using a property in the default implementation of a trait's function.

@MolotovCherry
Copy link

MolotovCherry commented Sep 19, 2022

Would love this feature. Would greatly improve readability in many cases, instead of having to do so many getter/setter methods just to access some fields. I feel getter/setter methods have their own place and a reason (if there's more complicated workings going on there, or if you need to hide implementation details), but sometimes all you need/want is simple field access

Reading the comments here, it seems there might be a lot to change to support such a thing.

@dbsxdbsx
Copy link

@nikomatsakis, may I ask if this feature is totally banned or just postponed?

@NightFeather0615
Copy link

Another bump, any update for this?

@burdges burdges mentioned this pull request Feb 5, 2023
@Yabgu
Copy link

Yabgu commented Mar 14, 2023

I found here reading why-im-dropping-rust. Looks like Rust is hitting the same issues with C++. I remember people promoting named-parameter-idiom in C++ way back then, to either delay or reject designated initializers... I always wished I could use designated initializers and most of the time I had to move to C because... (long list of reasons),

It looks like either Rust language designers really love getter methods in a similar way the C++ committee loved named-parameter-idioms and/or there is a core design philosophy is really contradicting this proposal; If either is true, then even if you get this thing let's say 2033, it will be a similar story. Also don't buy, too complex or backward compatibility, this is not a workload issue, this is a decision and direction.

@ahicks92
Copy link

Actually, the primary point of this is to change how the lifetime system works by exposing struct splitting at the language level (in effect), and is significant work because rewriting the lifetime system to handle it and many other things is taking ongoing experimental work. Others can provide more details; I'm more divorced from the compiler's ongoings than I used to be. It's not as simple as "this abstracts over a pointer", and the primary point isn't that getters are annoying, it's that this lets the compiler understand when two references don't overlap.

I think this is valuable too, though, don't get me wrong. Just not as easy as it at first appears. If it was as simple as getters vs. not-getters you could solve it with macros or various hacks like what readonly does under the hood.

@ssh352
Copy link

ssh352 commented Mar 31, 2023

Would allowing fields in trait make the trait be like Java abstract class? Is Rusting on the route to have inheritance?

Can we just use macros to generate the accessor methods in implementing types?

@ssh352
Copy link

ssh352 commented Mar 31, 2023

But if this feature is done, does it mean that it is a kind of inheritance mechanism? Seems that Rust team don't like inheritance. And for most situation, composition is better than inheritance.

same question/doubt here.

@Yabgu
Copy link

Yabgu commented Mar 31, 2023

The comment below is kind of proof that it is intended this way. The Rust community wants to avoid the mistakes of C++, but they have a similar mindset. This is about having a feature like concepts even C++ has now: https://stackoverflow.com/questions/60434579/using-concepts-for-checking-if-a-type-t-has-a-field-f Example code they have looks like below:

void doSomething(auto i) requires requires { i.foo; } { /* */ }

If you ask me this looks much more like generic programming than Java-like inheritance.

But I guess high probably it is right, Rust should never have this feature, or it may be a waste of effort, or it smells like Java.
In meantime yes you can generate getters setters from templates which is what Lombok does in Java also the same as C# properties, but that is good,

@dbsxdbsx
Copy link

dbsxdbsx commented Apr 3, 2023

For those who just want to avoid redundant code of a FIXED but mutable struct field, you can work around with this:

trait MyTrait {
    fn get_field_a(&self) -> bool;
}

macro_rules! add_my_trait_impl {
    ($struct_name:ident { $($field_name:ident : $field_type:ty),* }) => {
        #[derive(Debug)]
        struct $struct_name {
            $($field_name : $field_type,)*
            a: bool,
        }

        impl MyTrait for $struct_name {
            fn get_field_a(&self) -> bool {
                self.a
            }
        }
    }
}

add_my_trait_impl! {MyStruct2 {}}
add_my_trait_impl!(MyStruct1 { x: i32, y: i32 });

fn main() {
    let my_struct1 = MyStruct1 {
        x: 1,
        y: 2,
        a: true,
    };
    let my_struct2 = MyStruct2 { a: false };
    println!("{:?}", my_struct1.get_field_a());
    println!("{:?}", my_struct2.get_field_a());
}

In this way, you don't have to write redundant code for get_field_a(), but you have to make sure there is ALWAYS a field called a:bool in each customized struct, which need this trait.

@RLC92
Copy link

RLC92 commented Jan 5, 2025

This is what specifically has driven me away from Rust.

I personally do not need inheritance, the ability to define macro fields inline would be enough. e.g.

struct Example {
some_macro_that_injects_fields!()
}

I'm not going to write the same 10 fields 10000 times and I'm not going to write insane boilerplate implementations over and over again.

@ssokolow
Copy link

ssokolow commented Jan 5, 2025

@RLC92 Have you tried crates like https://lib.rs/crates/delegate?

The general approach to getting things like this into Rust proper tends to be "wait for usage numbers of some workaround on crates.io to prove sufficient real-world demand".

@RLC92
Copy link

RLC92 commented Jan 5, 2025

@ssokolow I had seen crates like that as well as ones like hereditary, but everything seems geared towards methods which I do not really need, I have no behaviors associated with these fields, this is more about building an ontological representation in structs, I actually had a fairly good implementation going using an inner component but it became clear that was quickly going to become very annoying due to all the boilerplate, especially when all I need is a field. These fields aren't backed by anything, there are no methods, if the config loads and the tests pass then the program has done it's job, I'm just trying to represent an existing yaml and I don't understand why I can't just say "I want these fields here" without writing out every single one by hand. Terraform has better config building and it's terraform. I even took a look at proc macros but if something this simple requires so much boilerplate maybe it's too soon to jump on or maybe rust is not the right tool for this use case.

Regardless it feels like something is missing in the language.

Essentially all I need is:

Command Line command -> Loads and validates -> Sends it somewhere else.

But building the actual config to be represented by Serde is a PITA because the config has lots of duplicated elements (Because the user edits the config, most of which is generated, and these edits are essentially what we want to send)

I just don't think complicated structs with high duplication are handled well in rust.

@ahicks92
Copy link

ahicks92 commented Jan 5, 2025

@RLC92
Serde handles this via serde(flatten) as I recall: you make a struct with your shared fields, then embed it in another struct. E.g.:

#[derive(Deserialize)]
struct Shared {
    username: String,
}

#[derive(Deserialize)]
struct Message {
    #[serde(flatten)]
    shared: Shared,
    password: String,
}

Which can handle:

username: bob
password: foo

And if you want extra fields to be captured you can flatten whichever yaml crate's dynamic value in because flatten works on anything that is "map-like" (e.g. hashmaps). So for example, with serde_json:

#[derive(Deserialize)]
struct Message {
    special_field: String,
    super_special_field: String,
    #[serde(flatten)]
    other_junk: Value,
}

I have only once seen something Serde can't handle, and it was an internal format that nothing could handle. Even then though Serde was used for 75% of it. Suffice it to say friends don't let friends develop custom compressed JSON encodings for database tables. If you want to adopt Rust you might want to try any of the various avenues to ask questions about your problem before giving up. In the particular case of serde the ecosystem is quite mature, there's just a lot in it. For example a next obvious direction is "what about generics" and Serde can also flatten generic parameters in no problem, or "what about messages without an explicit type field" and Serde handles that no problem too. If that's not good enough many of the deserialization crates provide equivalents to Python-style x["foobar"] sometimes literally with that exact syntax where you get a panic at runtime, and helpers to check if values are present.

In any case this RFC wouldn't solve your problem. The trait doesn't define the fields, it defines pointers to the fields. You'd still have to declare the fields in the struct. This would add more code to your usecase not less. It also couldn't play nice with generic serialization libraries for a variety of reasons.

The ability to expand macros into fields in a struct would actually be really cool, though. I'm not sure why we don't have it, and I can think of obvious usage-related problems e.g. you can't generically access those fields without using macros or this RFC. I think it's pretty easy to expand the places macros are allowed and until today I'd have honestly thought I could use one there myself; I just know Rust well enough that it's not come up, because I know how to solve your problem the "right" way. I'm no longer close enough to the processes to say for sure but that seems like it would be a step where the usefulness is in line with the difficulty of doing it: maybe it's not that useful but I don't think it's that hard. Someone would have to put it through the RFC process however.

@RLC92
Copy link

RLC92 commented Jan 6, 2025

Problem solved in 5 minutes in python.

Serde does not solve the problem as it still leaves a terrible api for interacitng with these structs which means extracting snippets to send only the snippets that changed is still very unpleasant. I will not write bad code, not today, not yesterday, not tomorrow. And nesting a bunch of complex objects to end up with a.b.c^32 is just not rational. I'd be better of using dot notation keys in a hashmap if I was going to do something that atrocious.

I spent literally days on something that shouldn't take 5 minutes, don't insult me by implying I didn't try. It's clear rust doesn't want fields to be reused. There is no good solution that will not produce terrible code.

Essentially if you have highly repetitive nested structs, simply avoid rust.

@ahicks92
Copy link

ahicks92 commented Jan 6, 2025

Rust isn't for everyone, nor is this the place to be generally mad about Rust. Nor is what I proposed "bad code". If you are thinking I would be writing foo.shared.whatever all over this is not the case because there are at least 3 ways to solve that problem. But this also isn't the appropriate place to get into this debate, and if you are happy with Python and don't need the performance of Rust that's totally fine. Something I see a lot of people fail to understand is that Rust isn't for everything; even as a strong advocate for it who has brought it into an organization I will 100% say it's not universally the right tool. But still let's not go around bashing other programming languages especially in places where people who know that language well could solve the problem in 5 minutes with the tools you haven't learned. I find myself torn between just dropping it because this is an RFC thread, and feeling the need to respond to someone coming into an RFC thread, getting an answer to their question, calling that answer bad code, and telling us all the language sucks. Hopefully this response is a good compromise, and I am dropping it here.

@clarfonthey
Copy link
Contributor

Just want to comment that replying to decades-old RFCs is not a good way to propose or discuss features, since all it does is ping folks who were involved back then and can't easily get new folks involved. I'd recommend opening up a thread on Zulip or Internals instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. postponed RFCs that have been postponed and may be revisited at a later time. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.