rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
95.09k stars 12.26k forks source link

Tracking Issue for function delegation (`fn_delegation`, RFC 3530) #118212

Open petrochenkov opened 7 months ago

petrochenkov commented 7 months ago

This is a tracking issue for the RFC "Implement function delegation in rustc" (https://github.com/rust-lang/rfcs/pull/3530). The feature gate for the issue is #![feature(fn_delegation)].

The feature was accepted for experimentation in https://github.com/rust-lang/rust/pull/117978#issuecomment-1823066970.

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

Many of them in the RFC draft.

Implementation history

https://github.com/rust-lang/rust/pulls?q=is%3Apr+label%3AF-fn_delegation

petrochenkov commented 7 months ago

cc @Bryanskiy

jdonszelmann commented 1 month ago

I was looking through the code of the experimental implementation today (#117978), and I found something that surprised me. The experimental implementation of delegation has an id in it: image

However, a Delegation is used as an ItemKind, and any ItemKind is associated with an Item. Now every Item already has an ID associated with it: image

I looked through all the code, and I think for delegation the Item id is never used, and the Delegation specific one is always used but seems redundant.

Let me know if I'm misunderstanding something here, maybe I am, but I just wanted to let you know in case it's redundant.

petrochenkov commented 1 month ago

@jdonszelmann In

trait Trait { fn method(); }

impl Trait for Struct {
  reuse some::method;
}

the same Resolver::partial_res_map table keeps resolutions for both some::method and Trait::method (pointing from the impl to the trait). The former uses the Delegation's id as a key, and the latter uses Item's id as a key, so they need different ids.

The Delegation's id can be removed if a separate table is added for associated item resolutions in impls, instead of reusing partial_res_map.

petrochenkov commented 1 month ago

I'll update the list of subtasks in the issue a bit later, but for now I'll just post one of them for @Bryanskiy .

Suppose we want to delegate implementation of a whole trait with 10 methods, 5 of them are by ref and 5 are by mutable ref.

struct Wrapper(Inner);

impl Trait for Wrapper {
  fn ref1() { Trait::ref1(&self.0) }
  fn ref2() { Trait::ref2(&self.0) }
  fn ref3() { Trait::ref3(&self.0) }
  fn ref4() { Trait::ref4(&self.0) }
  fn ref5() { Trait::ref5(&self.0) }

  fn mut1() { Trait::mut1(&mut self.0) }
  fn mut2() { Trait::mut2(&mut self.0) }
  fn mut3() { Trait::mut3(&mut self.0) }
  fn mut4() { Trait::mut4(&mut self.0) }
  fn mut5() { Trait::mut5(&mut self.0) }
}

Right now we cannot do it using glob delegation and an explicit path, due to the difference between &self.0 and &mut self.0. We have to write

impl Trait for Wrapper {
  reuse Trait::* { &self.0 }
  reuse Trait::{mut1, mut2, mut3, mut4, mut5} { &mut self.0 }
}

Ideally we would use just

impl Trait for Wrapper {
  reuse Trait::* { self.0 }
}

and self.0 would autoref/autoderef/coerce automatically.

For that in the generated body Trait::method(self.0) we should treat self.0 as if it was a method receiver in a method call, rather than a regular argument. The difference with existing method calls is that in self.0.method() the method is resolved as a part of applying conversions to self.0, but in our case it is already pre-resolved to Trait::method. The types team should know how to do it correctly.

(This was mentioned in the RFC in application to a different case - https://github.com/petrochenkov/rfcs/blob/delegation/text/0000-fn-delegation.md#the-proposed-algorithm.)

jdonszelmann commented 1 month ago

@petrochenkov that makes sense. Would you like it if I gave it a comment or a slightly more descriptive name?

petrochenkov commented 1 month ago

The separate table approach may actually be a better way.