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} }
}

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

impl ops::AddAssign for Vec2D {
    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]
fn add_move_both() {
    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]
fn add_assign_move_rhs() {
    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 impls 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} }
}

impl<R:Borrow<Vec2D>> ops::AddAssign<R> for Vec2D {
    fn add_assign(&mut self, other: R) {
        let other = other.borrow();
        self.x += other.x;
        self.y += other.y;
    }
}

impl<R> ops::Add<R> for Vec2D where Vec2D: ops::AddAssign<R> {
    type Output=Vec2D;
    fn add(mut self, other:R)->Self::Output {
        self += other; self
    }
}

impl<'a, R> ops::Add<R> for &'a Vec2D where Vec2D: ops::Add<R> {
   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