Skip to content

Conversation

fsitok
Copy link

@fsitok fsitok commented Jun 9, 2023

The core fix makes a case distinction when the delegate_expr is self.
I'm not sure if the below check is the best way to check for equality to self. But it works.

+                    if quote!(#delegate_expr).to_string() == "self" {
+                        Ok(quote! { #(#arg_attrs)* #delegate_expr })
+                    } else {
+                        Ok(quote! { #(#arg_attrs)* #ref_ #mut_ #delegate_expr })
+                    }

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (72288e2) 10.05% compared to head (b55bbc8) 10.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master       #7   +/-   ##
=======================================
  Coverage   10.05%   10.05%           
=======================================
  Files          12       12           
  Lines         577      577           
=======================================
  Hits           58       58           
  Misses        519      519           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@SOF3
Copy link
Owner

SOF3 commented Jun 10, 2023

Making self a special case feels a bit hack tbh. I'm wondering if there is a more generic syntax, e.g. allowing user to specify different expressions for different receivers.

@fsitok
Copy link
Author

fsitok commented Jun 13, 2023

You're right. Figured out it is also not necessary. Using *self as a delegate expression seems to do the job.
Added this example as a testcase.

Potentially interesting could be to support two delegate expressions. An immutable one and a mutable one.
This would for example allow to delegate to attributes wrapped into an RwLock<_>.

@SOF3
Copy link
Owner

SOF3 commented Jun 13, 2023

@fsitok Using *self doesn't work if a function contains both self and &self receivers, right?

@SOF3
Copy link
Owner

SOF3 commented Jun 13, 2023

The difference between self and self.field is that the former could have different types depending on the receiver, but the latter is always dereferenced due to the . operator.

One alternative solution I thought of is to replace all self tokens to (*self) when the receiver takes by (shared/mutable) reference so that it works like this snippet:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d91ba50237c36a928214a0a0da2d08d7

The problem is that, in this case, self cannot be used in any generic case, because (&(*self)).foo() works, but let this = *self; (&this).foo() wouldn't work, so we still have some sort of inconsistency.

@SOF3
Copy link
Owner

SOF3 commented Jun 13, 2023

So far we have just dealt with delegation targets in the form self and self.field. What if the delegation target is some_fn(self) (where some_fn accepts a generic type that may or may not be a reference)? Do we need to support such use cases?

@SOF3
Copy link
Owner

SOF3 commented Jul 25, 2023

An alternative solution is to use destructuring. If we somehow write

let Self { field, .. } = self;

then it does not require any special handling for references, although it's specific to field support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants