&self/&mut self in traits considered harmful(?)

classic Classic list List threaded Threaded
65 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

SiegeLord
First, let me begin with a small discussion about C++ rvalue references.
As some of you know, they were introduced to C++ in part to solve
problems like this:

Matrix m;
m.data = {1.0, 2.0, 3.0};
Matrix m2 = m * 2.0 * 5.0 * 10.0;

Before C++11, most implementations the multiplications on the third line
would create two (unnecessary) temporary copies of the Matrix, causing
widespread inefficiency if Matrix was large. By using rvalue references
(see the implementation in this gist:
https://gist.github.com/SiegeLord/85ced65ab220a3fdc1fc we can reduce the
number of copies to one. What the C++ does is that the first
multiplication (* 2.0) creates a copy of the matrix, and the remaining
multiplications move that copy around.

If you look at the implementation, you'll note how complicated the C++
move semantics are compared to Rust's (you have to use std::move
everywhere, define move-constructors and move-assignment with
easy-to-get-wrong implementations etc.). Since Rust has simpler move
semantics, can we do the same thing in Rust?

It turns out we cannot, because the operator overloading in Rust is done
by overloading a trait with a method that takes self by reference:

pub trait Mul<RHS, Result>
{
     fn mul(&self, rhs: &RHS) -> Result;
}

This means that the crucial step of moving out from the temporary cannot
be done without complicated alternatives (explained at the end of this
email). If we define an a multiplication trait that takes self by value,
however then this is possible and indeed relatively trivial (see
implementation here:
https://gist.github.com/SiegeLord/11456760237781442cfe ). This code will
act just like the C++ did: it will copy during the first move_mul call,
and then move the temporary around:

let m = Matrix{ data: vec![1.0f32, 2.0, 3.0] };
let m2 = (&m).move_mul(2.0).move_mul(5.0).move_mul(10.0);

So there's nothing in Rust move semantics which prevents this useful
pattern, and it'd be possible to do that with syntax sugar if the
operator overload traits did not sabotage it. Pretty much all the
existing users (e.g. num::BigInt and sebcrozet's nalgebra) of operator
overloading traits take the inefficient route of creating a temporary
copy for each operation (see
https://github.com/mozilla/rust/blob/master/src/libnum/bigint.rs#L283 
and
https://github.com/sebcrozet/nalgebra/blob/master/src/structs/dmat.rs#L593 
). If the operator overloading traits do not allow you to create
efficient implementations of BigNums and linear algebra operations, the
two use cases why you'd even *have* operator overloading as a language
feature, why even have that feature?

I think this goes beyond just operator overloading, however, as these
kinds of situations may arise in many other traits. By defining trait
methods as taking &self and &mut self, we are preventing these useful
optimizations.

Aside from somewhat more complicated impl's, are there any downsides to
never using anything but by value 'self' in traits? If not, then I think
that's what they should be using to allow people to create efficient
APIs. In fact, this probably should extend to every member generic
function argument: you should never force the user to tie their hands by
using a reference. Rust has amazing move semantics, I just don't see
what is gained by abandoning them whenever you use most traits.

Now, I did say there are complicated alternatives to this. First, you
actually *can* move out through a borrowed pointer using
RefCell<Option<T>>. You can see what this looks like here:
https://gist.github.com/SiegeLord/e09c32b8cf2df72b2422 . I don't know
how efficient that is, but it is certainly more fragile. With my
by-value MoveMul implementation, the moves are checked by the
compiler... in this case, they are not. It's easy to end up with a
moved-out, dangling Matrix. This is what essentially has to be done,
however, if you want to preserve the general semantic of the code.

Alternatively, you can use lazy evaluation/expression templates. This is
the route I take in my linear algebra library. Essentially, each
operation returns a struct (akin to what happens with many Iterator
methods) that stores the arguments by reference. When it comes time to
perform assignment, the chained operations are performed element-wise.
There are no unnecessary copies and it optimizes well. The problem is
that its a lot more complicated to implement and it pretty much forces
you to use interior mutability (just Cell this time) if you don't want a
crippled API. The latter bit introduces a whole slew of subtle bugs (in
my opinion they are less common than the ones introduced by RefCell).
Also, I don't think expression templates are the correct way to wrap,
e.g., a LAPACK library. I.e. they only work well when you're
implementing the math yourself which is not ideal for the more
complicated algorithms. Along the same lines, it is not immediately
obvious to me how to extend this lazy evaluation idea to something like
num::BigInt. So far, it seems like lazy evaluation will force dynamic
dispatch in that case which is a big shame (i.e. you'd store the
operations in one array, arguments in another and then play them back at
the assignment time).

So, I think the situation is pretty bad. What can be done to fix it?

-SL

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Sebastian Gesemann
On Wed, Jun 11, 2014 at 3:27 PM, SiegeLord wrote:
> [...] Along the same lines, it is not immediately obvious
> to me how to extend this lazy evaluation idea to something like num::BigInt.
> So far, it seems like lazy evaluation will force dynamic dispatch in that
> case which is a big shame (i.e. you'd store the operations in one array,
> arguments in another and then play them back at the assignment time).

I havn't tried something like expression templates in Rust yet. How
did you come to the conclusion that it would require dynamic dispatch?

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Huon Wilson
In reply to this post by SiegeLord
On 11/06/14 23:27, SiegeLord wrote:
> Aside from somewhat more complicated impl's, are there any downsides
> to never using anything but by value 'self' in traits?

Currently trait objects do not support `self` methods (#10672), and,
generally, the interactions with trait objects seem peculiar, e.g. if
you've implemented Trait for &Type, then you would want to be coercing a
`&Type` to a `&Trait`, *not* a `&(&Type)` as is currently required.

However, I don't think these concerns affect the operator overloading
traits.


https://github.com/mozilla/rust/issues/10672


Huon


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

SiegeLord
In reply to this post by Sebastian Gesemann
On 06/11/2014 10:10 AM, Sebastian Gesemann wrote:
> On Wed, Jun 11, 2014 at 3:27 PM, SiegeLord wrote:
>> [...] Along the same lines, it is not immediately obvious
>> to me how to extend this lazy evaluation idea to something like num::BigInt.
>> So far, it seems like lazy evaluation will force dynamic dispatch in that
>> case which is a big shame (i.e. you'd store the operations in one array,
>> arguments in another and then play them back at the assignment time).
>
> I havn't tried something like expression templates in Rust yet. How
> did you come to the conclusion that it would require dynamic dispatch?

It's just the first idea I had with how this could work, but you're
right, I can envision a way to do this without using dynamic dispatch.
It'd look something like something like this:
https://gist.github.com/SiegeLord/f1af81195df89ec04d10 . So, if nothing
comes out of this discussion, at least you'd be able to do that. Note
that the API is uglier, since you need to call 'eval' explicitly.
Additionally, you need to manually borrow 'm' because you can't specify
a lifetime of the &self argument in mul (another problem with
by-ref-self methods).

-SL


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
If the `Mul` trait and similar were changed to take `self` by value, perhaps the following kind of language design would make more sense:

If a variable of a type that has a destructor is passed to a function by value (moved), and the variable is used after the function call, the variable would be implicitly cloned before passing it to the function.


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Daniel Micay
On 11/06/14 01:54 PM, Tommi wrote:
> If the `Mul` trait and similar were changed to take `self` by value, perhaps the following kind of language design would make more sense:
>
> If a variable of a type that has a destructor is passed to a function by value (moved), and the variable is used after the function call, the variable would be implicitly cloned before passing it to the function.

Cloning big integers, rationals based on big integers or arbitrary
precision floating point values for every single operation has a high
cost. One of Rust's strength's is that it doesn't have implicit cloning
as C++ does due to copy constructors.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://mail.mozilla.org/pipermail/rust-dev/attachments/20140611/bdbe23e1/attachment.sig>

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
On 2014-06-11, at 21:33, Daniel Micay <danielmicay at gmail.com> wrote:

> Cloning big integers, rationals based on big integers or arbitrary
> precision floating point values for every single operation has a high
> cost.

I didn't say that all functions should start taking their arguments by value. I said `Mul` and similar should do it, i.e. functions that take a variable and return a variable of that same type. Instead of passing by reference and making a clone of the passed reference inside those functions, you force the caller to make the clone and mutate the passed argument in place. This enables the C++ like rvalue reference optimization for functions like multiplication.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rust-dev/attachments/20140611/c93cf7ab/attachment.html>

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Corey Richardson
Keeping in mind that the `self` value here can be a reference. Ie,
implementing the traits also for references to a type.

On Wed, Jun 11, 2014 at 11:47 AM, Tommi <rusty.gates at icloud.com> wrote:

> On 2014-06-11, at 21:33, Daniel Micay <danielmicay at gmail.com> wrote:
>
> Cloning big integers, rationals based on big integers or arbitrary
> precision floating point values for every single operation has a high
> cost.
>
>
> I didn't say that all functions should start taking their arguments by
> value. I said `Mul` and similar should do it, i.e. functions that take a
> variable and return a variable of that same type. Instead of passing by
> reference and making a clone of the passed reference inside those functions,
> you force the caller to make the clone and mutate the passed argument in
> place. This enables the C++ like rvalue reference optimization for functions
> like multiplication.
>
>
> _______________________________________________
> Rust-dev mailing list
> Rust-dev at mozilla.org
> https://mail.mozilla.org/listinfo/rust-dev
>



--
http://octayn.net/

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
In reply to this post by Tommi Tissari
On 2014-06-11, at 21:47, Tommi <rusty.gates at icloud.com> wrote:

> I said `Mul` and similar should do it, i.e. functions that take a variable and return a variable of that same type.


Although, a larger issue of genericity is that multiplication doesn't always return the same type as one of its arguments.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rust-dev/attachments/20140612/804d4c01/attachment.html>

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
In reply to this post by SiegeLord
On 2014-06-11, at 16:27, SiegeLord <slabode at aim.com> wrote:

> So, I think the situation is pretty bad. What can be done to fix it?

I agree that this seems like a serious regression from C++. If it won't be fixed, I think I'll rather stick with C++. Better the devil you know...

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rust-dev/attachments/20140612/d157edc3/attachment.html>

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Patrick Walton-2
On 6/12/14 1:02 AM, Tommi wrote:
> On 2014-06-11, at 16:27, SiegeLord <slabode at aim.com
> <mailto:slabode at aim.com>> wrote:
>
>> So, I think the situation is pretty bad. What can be done to fix it?
>
> I agree that this seems like a serious regression from C++. If it won't
> be fixed, I think I'll rather stick with C++. Better the devil you know...

This message is unhelpful.

Patrick


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Patrick Walton-2
In reply to this post by Huon Wilson
On 6/11/14 7:17 AM, Huon Wilson wrote:
> On 11/06/14 23:27, SiegeLord wrote:
>> Aside from somewhat more complicated impl's, are there any downsides
>> to never using anything but by value 'self' in traits?
>
> Currently trait objects do not support `self` methods (#10672)

We'll have to fix this for unboxed closures, so it's a 1.0 thing.

Patrick

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Patrick Walton-2
In reply to this post by SiegeLord
On 6/11/14 6:27 AM, SiegeLord wrote:
> So, I think the situation is pretty bad. What can be done to fix it?

Seems to me we can just make the overloaded operator traits take
by-value self.

Patrick


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
In reply to this post by Patrick Walton-2
On 2014-06-12, at 19:05, Patrick Walton <pcwalton at mozilla.com> wrote:

> On 6/12/14 1:02 AM, Tommi wrote:
>> On 2014-06-11, at 16:27, SiegeLord <slabode at aim.com
>> <mailto:slabode at aim.com>> wrote:
>>
>>> So, I think the situation is pretty bad. What can be done to fix it?
>>
>> I agree that this seems like a serious regression from C++. If it won't
>> be fixed, I think I'll rather stick with C++. Better the devil you know...
>
> This message is unhelpful.

My post was potentially helpful in conveying the seriousness of this issue to someone who might not have considered it serious. I thought of it as advertisement for an important thread that wasn't getting much traction.


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
In reply to this post by Patrick Walton-2
On 2014-06-12, at 19:08, Patrick Walton <pcwalton at mozilla.com> wrote:

> On 6/11/14 6:27 AM, SiegeLord wrote:
>> So, I think the situation is pretty bad. What can be done to fix it?
>
> Seems to me we can just make the overloaded operator traits take by-value self.

I definitely wouldn't want to see something like the following:

pub trait GreaterByOne<T> {
    fn greater_by_one(self) -> Self;
}

pub fn my_algorithm<X, T: GreaterByOne<X> + Add<T, T>>(value: T) -> T {
    value.greater_by_one() +
    value.greater_by_one() // error: use of moved value: `value`
}

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rust-dev/attachments/20140612/d8cbca6a/attachment.html>

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Patrick Walton
You could just clone the value to get around that error.

On June 12, 2014 10:03:40 AM PDT, Tommi <rusty.gates at icloud.com> wrote:

>On 2014-06-12, at 19:08, Patrick Walton <pcwalton at mozilla.com> wrote:
>
>> On 6/11/14 6:27 AM, SiegeLord wrote:
>>> So, I think the situation is pretty bad. What can be done to fix it?
>>
>> Seems to me we can just make the overloaded operator traits take
>by-value self.
>
>I definitely wouldn't want to see something like the following:
>
>pub trait GreaterByOne<T> {
>    fn greater_by_one(self) -> Self;
>}
>
>pub fn my_algorithm<X, T: GreaterByOne<X> + Add<T, T>>(value: T) -> T {
>    value.greater_by_one() +
>    value.greater_by_one() // error: use of moved value: `value`
>}

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rust-dev/attachments/20140612/e17f8433/attachment.html>

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Corey Richardson
Or bound by Copy.

On Thu, Jun 12, 2014 at 10:17 AM, Patrick Walton <pwalton at mozilla.com> wrote:

> You could just clone the value to get around that error.
>
>
> On June 12, 2014 10:03:40 AM PDT, Tommi <rusty.gates at icloud.com> wrote:
>>
>> On 2014-06-12, at 19:08, Patrick Walton <pcwalton at mozilla.com> wrote:
>>
>> On 6/11/14 6:27 AM, SiegeLord wrote:
>>
>> So, I think the situation is pretty bad. What can be done to fix it?
>>
>>
>> Seems to me we can just make the overloaded operator traits take by-value
>> self.
>>
>>
>> I definitely wouldn't want to see something like the following:
>>
>> pub trait GreaterByOne<T> {
>>     fn greater_by_one(self) -> Self;
>> }
>>
>> pub fn my_algorithm<X, T: GreaterByOne<X> + Add<T, T>>(value: T) -> T {
>>     value.greater_by_one() +
>>     value.greater_by_one() // error: use of moved value: `value`
>> }
>>
>
> --
> Sent from my Android phone with K-9 Mail. Please excuse my brevity.
>
> _______________________________________________
> Rust-dev mailing list
> Rust-dev at mozilla.org
> https://mail.mozilla.org/listinfo/rust-dev
>



--
http://octayn.net/

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
In reply to this post by Tommi Tissari
I think a new keyword, something like `stable`, is needed for specifying that an argument passed to a trait function is guaranteed to be logically unchanged after the function call. For example:

trait Foo {
    fn foo(stable self);
}

impl Foo for int {
    fn foo(&self) {} // OK
}

impl Foo for uint {
    fn foo(self) {} // OK
}

impl Foo for Box<int> {
    fn foo(stable self) {} // OK (implicitly clones self)
}


fn main() {
    let x: Box<int> = box 42;
    x.foo(); // `x` is implicitly cloned
    x.foo(); // OK
}


Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Corey Richardson
It's called Copy. `trait Foo: Copy { ... }`.

On Thu, Jun 12, 2014 at 10:26 AM, Tommi <rusty.gates at icloud.com> wrote:

> I think a new keyword, something like `stable`, is needed for specifying that an argument passed to a trait function is guaranteed to be logically unchanged after the function call. For example:
>
> trait Foo {
>     fn foo(stable self);
> }
>
> impl Foo for int {
>     fn foo(&self) {} // OK
> }
>
> impl Foo for uint {
>     fn foo(self) {} // OK
> }
>
> impl Foo for Box<int> {
>     fn foo(stable self) {} // OK (implicitly clones self)
> }
>
>
> fn main() {
>     let x: Box<int> = box 42;
>     x.foo(); // `x` is implicitly cloned
>     x.foo(); // OK
> }
>
> _______________________________________________
> Rust-dev mailing list
> Rust-dev at mozilla.org
> https://mail.mozilla.org/listinfo/rust-dev



--
http://octayn.net/

Reply | Threaded
Open this post in threaded view
|

&self/&mut self in traits considered harmful(?)

Tommi Tissari
`Copy` types aren't really relevant to a discussion about adding to Rust the C++ like optimization of moving rvalues (of non-Copy types) when they're passed to certain functions.

On 2014-06-12, at 20:30, Corey Richardson <corey at octayn.net> wrote:

> It's called Copy. `trait Foo: Copy { ... }`.
>
> On Thu, Jun 12, 2014 at 10:26 AM, Tommi <rusty.gates at icloud.com> wrote:
>> I think a new keyword, something like `stable`, is needed for specifying that an argument passed to a trait function is guaranteed to be logically unchanged after the function call. For example:
>>
>> trait Foo {
>>    fn foo(stable self);
>> }
>>
>> impl Foo for int {
>>    fn foo(&self) {} // OK
>> }
>>
>> impl Foo for uint {
>>    fn foo(self) {} // OK
>> }
>>
>> impl Foo for Box<int> {
>>    fn foo(stable self) {} // OK (implicitly clones self)
>> }
>>
>>
>> fn main() {
>>    let x: Box<int> = box 42;
>>    x.foo(); // `x` is implicitly cloned
>>    x.foo(); // OK
>> }
>>
>> _______________________________________________
>> Rust-dev mailing list
>> Rust-dev at mozilla.org
>> https://mail.mozilla.org/listinfo/rust-dev
>
>
>
> --
> http://octayn.net/


1234