Github RFC: Delegation by elahn · Pull Request #2393 · rust-lang/rfcs · GitHub

Copy link

elahn commented on Apr 6, 2018

In the previous revision:

Delegation statements must be the first items in an impl block. There may be more than one delegation statement, but they must all be at the top.

Unresolved question: Is it useful and/or feasible to allow delegation statements to appear anywhere in the impl block, rather than all at the top?

@Centril: My gut feeling is that it is feasible. delegate to ; should be considered an item, and as such, you'd [rough sketch] first do a pass collecting all the items and after that you simply extract the delegate items and desugar those into a flat set, remove all the idents (LHS) found in the remaining set from the flat set, and then you finally merge the sets.

I think rustfmt should put them at the top, but people should be able to do as they like and the grammar should be flexible enough to accommodate that if it is technically possible.

@Ixrec: Based on my memory of past and current delegation threads:

1. Some people thought this was required to make the contextual keyword option viable. I think this line of reasoning predates epochs/editions. It's obviously obsolete now that we have official guidance on new keyword introduction.
2. When an impl block contains a delegation as well as a regular item implementation, it seems better style to put the delegation on top. Especially when it's a * delegation, since "Foo implements trait T by delegating almost everything to field f, except that method foo is ... " is the natural way to describe that sort of thing.

So imo, as part of editing this to say delegate should be a real keyword, we should also delete this restriction. Putting the delegate item(s) at the top is probably still good style, but there's no need to make it mandatory.

Unresolved question: Do we want to support all kinds of trait items, or should we be even more minimalist and support only methods in the first iteration?

@Centril: I think the initial set is conservative enough

These questions are considered resolved unless someone objects.

scottmcm commented on Apr 7, 2018

What happens when things go wrong? For example, what happens if I try to

impl Default for AddOnlyCounter {
    delegate * to 0;
}

Is there a way to map output (and output types)? For example, if I do

impl Add for AddOnlyCounter {
    delegate * to 0;
}

It seems like the output type is most likely u32, but I'd probably want it to be AddOnlyCounter. And what's the RHS? Does it just fail because the impl wants the RHS to be AddOnlyCounter, but self.0.add wants a u32?

In general, it seems like Self anywhere that's not also self is a landmine here.

petrochenkov commented on Apr 7, 2018

Yes, please (ignoring specific syntax).
This has potential to remove more useless boilerplate than all the "ergonomic initiative" RFCs combined.

Diggsey commented on Apr 8, 2018

It's possible I missed it, but there's no mention of delegating to nested fields, eg. delegate * to x.y - it seems... odd to constrain it to a direct field.

WiSaGaN commented on Apr 8, 2018

fn method = self.field.method; This syntax was suggested for delegating a single item. It’s not clear how to extend it to delegating multiple items, “most” items or all items in an impl.

I still prefer this syntax proposed by @eddyb . This syntax does not introduce new keyword (contextual or not). And the meaning is obvious, and "feels" just right. I would argue that we should not encourage "glob" delegations. I am wondering how many cases are left for "glob" if we can use impl MyTrait for MyType => self.0; to delegate all methods in a trait.

Ixrec commented on Apr 8, 2018

@WiSaGaN

I would argue that we should not encourage "glob" delegations.

Do you mean "glob delegation" as in any syntax to delegate everything not explicitly implemented, or just the use of * in the proposed syntax? I would've thought "glob delegation" meant the former, but you appear to be proposing that impl MyTrait for MyType => self.0; do exactly the same thing in the very next sentence.

WiSaGaN commented on Apr 8, 2018 •

@Ixrec the RFC states the shortcoming of fn method = self.field.method; syntax is the inability to handle multiple, most or all items in an impl. I assume this means the cases not covered by impl MyTrait for MyType => self.0;. In these cases, explicitly spelling out the functions one need instead of conveniently pulling in all functions not grouped by any traits seems desirable because it

• promotes conservatively exposing functions in the new type that reduces the possibility of accidentlly pulling in unintended functions
• minimizes reader/maintainer's search effort for what are delegated exactly

I'm happy that there are people working on this as I find the motivation compelling. But one of the reasons #1406 was closed was that it tried to do too much, with too much syntax. I think this RFC still has that same problem. Personally the only feature I want is to delegate an entire trait implementation to a field. An attribute would do fine, I see no need for first-class syntax.

Ixrec commented on Apr 8, 2018 •

@leodasvacas To clarify, are you only talking about the introduction of dedicated syntax, or do you also feel this RFC "does too much"? It sounds like you're saying both, but the actual functionality proposed here (i.e., ignoring all of the possible future extensions) is a small subset of what #1406 proposed and under-specified. And part of the reason #1406 was closed is because its proposed reuse of an existing keyword was potentially confusing and ambiguous; now that we have epochs/editions introducing a new keyword is intended to be an improvement on it (if you have a specific alternative to a new keyword without the problems of #1406 I'd love to hear it).

@WiSaGaN I honestly can't figure out what you're trying to say in that comment. Are you objecting to the feature of delegating all items? Or the feature of delegating most/all non-explicitly implemented items? Or some specific syntax for either or both of those? I believe impl MyTrait for MyType => self.0; syntax can handle the all case but not the most case, while I don't see how fn method = self.field.method; could handle either.

WiSaGaN commented on Apr 9, 2018

@Ixrec We may be able to use impl MyTrait for MyType => self.0; with fn method = self.field.method;. These two won't cover the case where a type has many inherent methods, and you want to delegate all of them in one line, and the "most" cases. By using fn method = self.field.method; in these cases, you typed more, but it has the two benefits listed in my previous comment.
This approach though, does not introduce "delegate", nor it introduces the "*".

clarfonthey commented on Apr 10, 2018 •

One other argument in favour of using self.field instead of field is this:

impl<'a, T: 'a + Trait> Trait for &'a T {
    delegate * to *self;
}

Which, IMHO, should be allowed from the start. So many traits do this and I think it's extremely reasonable to include it. Additionally, disallowing delegate * to self at least when deny(unconditional_recursion) is on is also super important; personally, I'd even say that this should be a hard error.

I think that it should be clarified that this syntax should not alter the behaviour compared to the manual equivalent. To the point where, if they differ, it should be considered a compiler bug. This goes either way; if something works with delegation (as it should) when it fails manually, I'd consider it a bug.

kornelski commented on Apr 10, 2018 •

Could I delegate Ord to a combination of two fields?

struct MyStruct {
   field_a_ignore_me: Foo,
   field_b: u8,
   field_c: String,
}

impl Ord for MyStruct {
    delegate * to |&self| (self.field_b, self.field_c);
}

Ixrec commented on Apr 10, 2018

@kornelski No. That probably falls under this item on the list of possible future extensions:

Delegating “multiple Self arguments” for traits like PartialOrd, so that delegate * to f; would desugar to something like self.f.partial_cmp(other.f)

Havvy commented on Apr 11, 2018

Nit: This is proposing a delegate associated item, not a statement.

elahn commented on Apr 12, 2018

@scottmcm: What happens when things go wrong? For example, what happens if I try to

impl Default for AddOnlyCounter {
    delegate * to 0;
}

A nice compiler error explaining Default isn't implemented for u32, so the implementation for items of Default cannot be delegated to AddOnlyCounter::0.

Is there a way to map output (and output types)? For example, if I do

impl Add for AddOnlyCounter {
    delegate * to 0;
}

It seems like the output type is most likely u32, but I'd probably want it to be AddOnlyCounter. And what's the RHS? Does it just fail because the impl wants the RHS to be AddOnlyCounter, but self.0.add wants a u32?

Since we're delegating to u32, the signature of u32.add() is used, so RHS is u32:

impl Add for AddOnlyCounter {
    type Output = <u32 as Add>::Output;

    fn add(self, other: u32) -> u32 { self.0.add(other) }
}

To map the output type using the possible future extension "delegate block":

impl Add for AddOnlyCounter {
    type Output = AddOnlyCounter;
    delegate * {
        |self| self.0
    } -> {
        |delegate| AddOnlyCounter { delegate }
    }
}

// Generated code:
impl Add for AddOnlyCounter {
    type Output = AddOnlyCounter;

    fn add(self, other: u32) -> AddOnlyCounter {
        AddOnlyCounter { self.0.add(other) }
    }
}

For RHS to be AddOnlyCounter, the possible future extension "delegating multiple Self arguments" would be needed.

mehcode commented on Apr 16, 2018 •

I think I'm missing the point here. Couldn't this be trivially provided by a crate and a proc macro for the common case? Does it really need to be syntax?

#[delegates(Trait, "bar")]
struct Foo { bar: Bar }

clarfonthey commented on Apr 17, 2018

@mehcode the proc macro would have to have the ability to tell what methods Trait has, so, it'd be extremely limited.

scottmcm commented on Apr 19, 2018

Meta-comment: I definitely want some form of delegation, but I think this RFC could still do a better job of carving out a simple and uncontroversial subset. For example, when I think of delegating Add for struct MyNewtype(u32), I expect MyNewtype: Add<MyNewtype, Output=MyNewtype>. This RFC doesn't do that. And that's fine; I'd just rather, then, that it was a "isn't supported for delegation at this time" error. Maybe a simple rule can be found for "delegation-safe" items (like there's a rule for "dyn-compatible traits) that this can be restricted to for now, with additional support added later for the tricky cases.

First Draft: Only methods that don't mention Self other than in self, &self, and &mut self.

A nice compiler error explaining Default isn't implemented for u32

Umm, u32 is Default: https://doc.rust-lang.org/beta/std/primitive.u32.html#impl-Default

@hobofan which exact concerns are there? I don't remember anything important, since it's just plain stupid forwarding.

I disagree that this can be done using macros. Macros can't see definitions of traits from other crates. I actually proposed to just expand macros but quickly found there's a significant problem: impossible to determine the order of macro expansion. This may be one of the reasons why ambassador is not popular - the interesting traits are missing. (I guess the strange name is another reason.)

That being said, I didn't have need for this for a wile except for impl fmt::Display for struct MyNewtype(String);, so you still may have point. I guess one of the reasons behind wanting it was wanting to silence people whining about lack of inheritance. So an alternative way to silence them is saying "Delegation is better and manual delegation is rare." I still might be completely wrong and people may manually delegate more than I do.

ibraheemdev commented on Nov 25, 2020 •

Well, take a look at the actix_web::HttpRequest:

pub struct HttpRequest(pub(crate) Rc<HttpRequestInner>);

impl HttpRequest {
    pub fn head(&self) -> &RequestHead {
        &self.0.head
    }

    pub fn match_pattern(&self) -> Option<String> {
        self.0.rmap.match_pattern(self.path())
    }

    pub fn match_name(&self) -> Option<&str> {
        self.0.rmap.match_name(self.path())
    }

    pub fn extensions(&self) -> Ref<'_, Extensions> {
        self.0.head.extensions()
    }
    // ...
}

This boilerplate could be removed with delegation, or struct embedding (#2431). I think some form of efficient-code-reuse is needed in a language without inheritance. Go handles this with embedding. Because Rust does not allow implementing external traits for external structs and instead encourages the newtype pattern, delegation/embedding makes sense in the language.

How do other general-use non OO languages (haskell, etc.) handle this?

hobofan commented on Nov 25, 2020 •

@Kixunil What are you goals here apart from discounting all the work that's been put into the RFC, by calling everything in it "bikeshedding", and the whole feature "plain stupid forwarding" (which it isn't)?

Merging RFC doesn't say anything about stabilization.

Merging a RFC normally indicates that a feature has generally been accepted for future implementation and all the major pitfalls have been ironed out (or will likely be ironed out along the way). Postponing that important work to a later stage, where there is usually less community involvement can lead to not-so-nice situations where the community feels like they've been locked out from important discussion (as has happened in the case of deciding on the .await syntax post-RFC). Just accepting an RFC without any intent to implement and stabilize it (especially if it might have design flaws) is a bad idea.

In the case of this RFC there doesn't seem to be a consensus on the syntax of the proposal. There also doesn't seem to be a consensus that the feature is important enough to make it into the language at all (if #2429 is any indication rather not). As @ibraheemdev correctly points out #2431 (which itself isn't even a proper RFC yet) might also be a viable alternative that might be preferable, but neither issues have received a lot of discussion since that one was opened.

I disagree that this can be done using macros.

While I don't particularly care about ambassador these days, I think one of the main points it proved is that it can be done with new mechanisms that have become available to the macro system since earlier discussion.

Macros can't see definitions of traits from other crates.

Ambassador has a mechanism specifically for that, and even for the specific fmt::Display case. As you correctly point out the "interesting traits" are missing, as I didn't bother to go through all the standard library traits and including them in some built-in way. IIRC there is nothing that prevents that from being done.

If you want to go really crazy you could possible forgo that whole mechanism and get to the trait information via other ways (which I don't really want to expand on, as I might be burned at the stake for doing so).

Ambassador has a mechanism specifically for that

Yes, but it isn't really an elegant mechanism. It requires copy-pasting code from another crate, and keeping it up to date.

I do think it would be good to have this done in macros, but a really nice solution would require macros to have more type information. Personally I would rather see movement towards having some sort of type-aware macro system than this specific use case.

burdges commented on Nov 26, 2020 •

I think struct embedding #2431 never became an RFC because it invites too many complications (semver, diamond pattern, enums, etc.) and never produced many useful examples.

Ambassador looks sufficient for delegation honestly. I suppose nobody asked for syntax like #[delegate(Trait, invoke = "method")] yet, which kinda answer a discussion above. I'm surprised #[delegatable_trait] does not require any options, very nice.

I think #[delegatable_trait_remote] looks fine now, but proc macros could implement a #[delegatable_trait_fetched(Trait)] that actually fetches and replicates the definition. But why bother?

I'd expect partial delegation like discussed above to be more useful than remote delgation, although if few people use delegation anyways.

#[delegatable_trait]
trait Foo {
    .. many methods ..
}

struct Bar { }
impl Foo for Bar { .. }

#[derive(Delegate)]
struct Baz { baz: Baz, .. }

#[delegate_partial]
impl Foo for Baz {
    .. just one method ..
}

It'd rock if rust provided some explicit macro cache, so you could read macro outputs under target/.

I think #[delegatable_trait_remote] looks fine now

Consider the case where you have a crate where you have a single struct (or enum) that you want to implement a large trait (like, say slog::Serializer ), that delegates every method to one of its fields. ambassador doesn't really buy you anything there, because you still have to provide the definition of the trait.

There are certainly cases where ambassador is useful, but I suspect it is fairly common where a crate only needs to delegate a trait for a single data type within that crate. And in that case ambassador doesn't not provide a significant benefit.

@hobofan hard to tell over text but it seems to me you had a bad day or something. That's fine, I'm not offended/affected. Just want to let you know I definitely didn't want to discount any work and I didn't call the RFC bikeshedding, but the discussion that proposes tons of various syntaxes. My idea is to get something working in nightly to see how it behaves in real life as that might actually help indecision if people are allowed to try it out.

Thanks for pointing out the issue with await. Is there a way we can fine-tune it? Perhaps giving people louder heads-up?

Could you explain what exactly is not simple about such forwarding? My thinking is if we could write a hypothetical macro that is guaranteed to be non-recursive, expanded last and can take definition of any item just by specifying the path of this item, then it could be done with such a macro.

I didn't bother to go through all the standard library traits and including them in some built-in way

And this is exactly the problem I'm talking about. Even you, the creator of crate supposed to address the problem, were not motivated to do the work. Such work is tedious and uninteresting and could be done by a machine - what this RFC is all about.

hobofan commented on Nov 26, 2020 •

@Kixunil

My idea is to get something working in nightly to see how it behaves in real life as that might actually help indecision if people are allowed to try it out.

Thanks for pointing out the issue with await. Is there a way we can fine-tune it?

The best way to avoid similar situations is to try out as much as possible in the compiler already. If someone wants to implement a prototype of this feature directly in the compiler for people here to try out and evaluate against different approaches, they are already free to do so right now, and that has been done for RFCs with similar syntactic concerns in the past. There is no need for an RFC to be accepted for someone to start doing so and on the flip side an implementation won't just start materializing in nightly by accepting an RFC unless there is someone motivated to implement the feature anyways.

Could you explain what exactly is not simple about such forwarding?

Of course the underlying mechanism is "just" forwarding, but I think the RFC already lays out all the non-simple parts about it (e.g. partial forwarding). That paired with arriving at a good syntax makes this feature non-trivial in my opinion. If one is already is set on the syntax as laid out in the RFC, it might indeed be straightforward to implement (I don't know enough about the compiler internals to judge that).

Even you, the creator of crate supposed to address the problem, were not motivated to do the work.

I'm in no way "supposed" to do that work. I wrote and published the crate mostly to show that previous limitations of the macro system can finally be worked around and to serve my use-case at the time that wasn't handled well by existing crates (delegating large traits through multiple levels). If there is a pressing problem to a user of the crate they are free to contribute that or even just open an issue for that. (In particular for your example of delegating std:: traits on newtype structs, I've always been satisfied with derive_more).

There is no need for an RFC to be accepted for someone to start doing so

Oh, I had the impression that RFC is required. Thanks for clarifying!

This comment has been minimized.

Kixunil commented on Jan 14

@hhggit could you also provide an example of case when there is an inherent fn and trait fn with the same name and when there are two or more trait fns with the same name?

I started a thread proposing a variation of "delegation" called "forwarding" (Pre-RFC: Forwarding) which is closer to the syntax suggested by @eddyb and @WiSaGaN. This is a very highly requested feature and that consensus seems to be that it makes sense for it to be included in the language. Hopefully we can decide on a path forward soon!

ibraheemdev commented 25 days ago •

I've been thinking about this issue more lately, and I've come to the decision that the design outlined by this RFC is probably the best way to go about it. It outlines a much simpler and straightforward solution to the problem than any other proposals I have seen (#493, #2431), although some of those proposals cover more use cases and should not be discounted. This issue comes up a lot from beginners. You explain to them that Rust favors composition over inheritance, and then they get confused as to why there is so much boilerplate. Someone wants to implement an external trait for an external type, and you tell them to new-type it, but then they lose all the other trait implementations and have to resort to copy pasting self.0.foo() everywhere, which again, is very confusing. For a language that argues against inheritance, it feels very odd that there is no solution to efficient code re-use. External crates like delegate cover some use cases, but delegating an external trait implementation is still only possible manually, or with hacks like ambassador's copy-paste solution, and again, it is confusing that Rust favors composition over inheritance, but you often still need an external library (both of which depend on syn - compile times) to get rid of boilerplate. Many jump to implementing Deref, which was not it's original purpose, and yet is recommended by the Rust book. Can we move forward with this RFC or get some triage from the lang team? I feel that this is a very important issue, but the discussion around it has died down. Rust 2021 edition is coming up soon, and this RFC introduces a new keyword (delegate), so it seems like a good time to get things started up again.

Kixunil commented 24 days ago

Wanted to help this move so I gave it another read. I agree that this proposal is very good. I'm just not sure about * delegating everything including types, while explicit syntax doesn't allow delegating types. I'm not convinced forbidding types at first is beneficial. I suspect the difficulty of implementing it is marginal.

nikomatsakis commented 12 days ago

@rfcbot fcp postpone

Hello everyone; we discussed this RFC in our backlog bonanza. The consensus was that we that we should postpone it, as we don't think we have the bandwidth to see to it right now, but we are very interested in this general direction, and we agree that this addresses some real problems in Rust.

We would like to encourage folks to discuss it when the time comes for us to discuss our upcoming roadmap (one of the procedural changes we have in mind is to make it clearer when we'd be open to bigger proposals).

Speaking for the team, I do want to apologize. I realize that we encouraged @elahn and others to do this design work, and then let it languish and I'm sorry about that. :(

rfcbot commented 12 days ago •

Team member @nikomatsakis has proposed to postpone this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot commented 12 days ago

This is now entering its final comment period, as per the review above.

nikomatsakis commented 12 days ago

I've checked the names of those folks who were present in the meeting.

rfcbot commented 2 days ago

The final comment period, with a disposition to postpone, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC is now postponed.