# Implementing ops traits

I am writing a type which should support an arithmetic operator via both `Op`
and `OpAssign`. Let's take `+` and `+=` as a concrete example.

My natural instincts make me want to ensure that all the following cases work:

`````` a +  b;
&a +  b;
a + &b;
&a + &b;

a +=  b;
a += &b
``````

Moving the LHS of `+=` makes no sense, so there are only 2 (rather than 4) cases
for `+=` and the borrow is implicit on the LHS.

It's pretty clear to me how to implement support for any single one of these
cases in isolation, but I have 2 major problems in the big picture:

1. The borrow and move overloads seem to interfere with each other in ways that
I don't fully understand.

2. I find myself unable to reuse the logic in some of the cases.

For the sake of illustration, let's take the concrete example of a 2-dimensional
vector. In C++ the whole lot can be implemented with something along the lines of

``````class Vec2D {
public:
Foo(float x, float y): x(x), y(y) {}

Foo& operator+=(const Foo& other) {
this->x += other.x;
this->y += other.y;
return *this;
}

friend Vec2D operator+(Vec2D lhs, const Foo& rhs) {
lhs += rhs; // reuse operator+= defined above
return lhs;
}
private:
float x;
float y;
};
``````

Points to note:

1. All 6 variants listed at the top of the question are covered by this implementation.
2. Only 2 functions need be defined to cover all the cases.
3. The core logic is implemented in only one of these functions and is reused in the other.

The Rust equivalent:

``````use std::ops;

#[derive(Debug, Clone, PartialEq)]
pub struct Vec2D {
x: f32,
y: f32,
}

impl Vec2D {
fn new(x: f32, y: f32) -> Self { Vec2D{x, y} }
}

type Output = Self;
fn add(self, other: Self) -> Self {
let mut res = self.clone();
res += other;
res
}
}

fn add_assign(&mut self, other: Self) {
self.x += other.x;
self.y += other.y;
}
}
``````

gets me as far as passing these two tests

``````#[test]
let a = Vec2D::new(2.0, 3.0);
let b = Vec2D::new(4.0, 5.0);
assert_eq!(a + b, Vec2D::new(6.0, 8.0));
}

#[test]
let mut a = Vec2D::new(2.0, 3.0);
let     b = Vec2D::new(4.0, 5.0);
a += b;
assert_eq!(a, Vec2D::new(6.0, 8.0));
}
``````

which cover only 2 of the 6 cases listed at the top of this question.

Trying to cater for the four remaining cases (those involving borrowing the
operands) leads to the conflicts and code duplication in mentioned above.

How should these operators be implemented in Rust?

I would probably approach it something like this, and consider writing a macro that could produce the bottom two `impl`s it I have a lot of them to write. Also, for something as small as `Vec2D`, I would probably derive `Copy` and never bother with shared referencesā on a 64-bit machine, itās the same size as a pointer.

``````use std::ops;
use std::borrow::Borrow;

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Vec2D {
x: u32,
y: u32,
}

impl Vec2D {
fn new(x: u32, y: u32) -> Self { Vec2D{x, y} }
}

fn add_assign(&mut self, other: R) {
let other = other.borrow();
self.x += other.x;
self.y += other.y;
}
}

type Output=Vec2D;
self += other; self
}
}

type Output = <Vec2D as ops::Add<R>>::Output;
fn add(self, other:R)->Self::Output { self.clone() + other }
}
``````
1 Like

Based on @2e71828's neat start point, I have written a macro that allows writing:

``````#[derive(Clone)] // <- required
pub struct Vec2D {
pub x: u32,
pub y: u32,
}

impl_all_ops! {
impl Add Assign? for &? Vec2D {
fn add_assign (self: &mut Self, other: &Self)
{
self.x += other.x;
self.y += other.y;
}
}
}

/// Check that all the desired ops work.
fn check (
mut a: Vec2D,
b: Vec2D,
)
{
#![allow(warnings)] // use a match to avoid worrying about ownership interactions.
match () {
_ => { a = a + b; },
_ => { a = &a + b; },
_ => { a = a + &b; },
_ => { a = &a + &b; },
_ => { a += b; },
_ => { a += &b; },
}
}
``````
3 Likes

Thanks, @2e71828 and @Yandros for the code samples.

I'm struggling to understand why @Yandros' macro implements `add` 3 times (in addition to a single implementation for `add_assign`, while @2e71828's plain code only needs 2 definitions of `add` (+ 1 `add_assign`).

I'm also having trouble with `self` seemingly playing different syntactic roles inside the macro, but I haven't formulated a clear question on this yet, I need to bang my head against it a bit more first. [edit: I think I more or less understand the `self`-related issues after reading Non-Identifier Identifiers in the The Little Book of Rust Macros.]

Oh I like that match trick. Make the borrow checker see parallel uses instead of serial. Clever. And I mean that as a good clever, not the bad kind.

Using macros is probably your best bet here, because yes there's a bunch of cases to handle -- see all the `Add` impls on `u32`, for example.

Here's a version of the macro I wrote a while back:

Which gets called later like

``````    impl_math_for_copy!(Add & AddAssign for \$name as add & add_assign +);
impl_math_for_copy!(Sub & SubAssign for \$name as sub & sub_assign -);
impl_math_for_copy!(Mul & MulAssign for \$name as mul & mul_assign *);
impl_math_for_copy!(Div & DivAssign for \$name as div & div_assign /);
``````
1 Like

Thank you all for your suggestions.

I've placed my solution in this playground.

The interface looks like this:

``````make_ops!(+ for SomeType: Clone (self, other) {
self.whatever += other.whatever;
});
``````
• Does this interface look reasonable to a Rustacean?

• My interface requires the user to specify explicitly whether the type is `Clone` or `Copy`. Is there a way for this to be detected automatically?

• Would it be useful to support types which are not copyable at all?

• Does everyone tend to roll their own, or is there a crate out there which supplies this sort of functionality? If not, would it be useful for there to be one?

All `Copy` types implement `Clone` as well, so if you have an implementation for `Clone`, thereās no need for a separate `Copy` one.

The interface looks fine, but youāll want to use fully-qualified paths for everything if this is going to be an exported macro vs. just something you use internally. That tends to reduce readability quite a lot, though, so I usually only do it as a last step after everything else is right.

1 Like

I appreciate that.

As I understand it, a type could fall into one of the following three categories

1. `Copy` + `Clone`
2. `Clone`
3. neither `Clone` nor `Copy`

In the first case `Copy` might be more efficient than `Clone` (but not less efficient), so implementing everything in terms of `Clone` simplifies the macro and the macro-writer's job, but has sub-optimal performance in these cases.

Hence, `Copy` should be used when available, even though falling back on `Clone` is semantically correct.

If there were a way for the macro to pick the best option automatically, rather than requiring the user to specify it, that would be great.

This is theoretically true, but I would be shocked if the generated code was any different in all but exceptional cases. Every `Clone` implementation I know of for a `Copy` type is trivial in practice.

Rust doesnāt provide any facilities for basing a decision on the absence of a trait (`Copy` in this case). I think it causes some undesirable semver hazards, but itās been a while since I looked into the reasoning.

That's correct. Rust never wants a library implementing a new trait to be a breaking change. (With a handful of exceptions, like `Drop` -- but it's a weird trait in so many ways anyway...)

It is expected that `Copy`-able types implement the `Clone` API by relying on a `Copy`:

``````/// Imagine we had the following blanket impl:
impl<T : Copy> Clone for T {
// default
fn clone (self: &'_ Self) -> Self
{
*self
}
}
``````

So calling `.clone()` on something `Copy`-able is the same as doing `*`.

(In practice, there could be somebody implementing `Clone` differently, but then that's their fault).

6 Likes

I notice that in most of the macro implementations that people have submitted in this topic, lifetimes are specified explicitly here and there. In writing and testing my solution I never came across the need to specify lifetimes explicitly.

What have I missed?

In mine, the only lifetime annotation is here:

I included it out of habit, mostly. `&T` as a type is fundamentally tied to a lifetime. Most of the time, youāre going to want a name for that lifetime to use inside the body on parameters and return types. I also think it may have been required at some point in the past (though Iām not sure).

Yes, elision in `impl` headers like that was new in 1.31 -- for a quick overview see the FCP comment for stabilization, https://github.com/rust-lang/rust/issues/15872#issuecomment-412759783

Or take a look at all the lifetimes it allows us to delete from libcore:

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.