2

Is there a way to have a merge function that uses &mut self, consume inner enum value and push it onto a new Vector when merging? I keep fighting compiler on this -- PEBKAC, but where?

If this is not possible as is, can it be fixed with implementing Clone trait on Val? (but NOT the Copy trait!)

struct Val();

enum Foo {
    One(Val),
    Many(Vec<Val>),
}

impl Foo {
    pub fn merge(&mut self, other: Self) {
        match (*self, other) {
//             ^^^^^
// move occurs because `*self` has type `Foo`, which does not implement the `Copy` trait
// cannot move out of `*self` which is behind a mutable reference
//

            (Self::One(a), Self::One(b)) => *self = Self::Many(vec![a, b]),
            (Self::One(a), Self::Many(mut b)) => {
                b.insert(0, a);
                *self = Self::Many(b)
            }
            (Self::Many(mut a), Self::One(b)) => {
                a.push(b);
            }
            (Self::Many(mut a), Self::Many(b)) => {
                a.extend(b);
            }
        };
    }
}
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
Yuri Astrakhan
  • 8,808
  • 6
  • 63
  • 97
  • I don't understand what you're trying to do. If you plan to consume `self`, then take it by value. If you want to replace its insides, then do that. Two of your three branches leave `self` in an uninitialized state, so even if Rust accepted this code, it would be unsafe Rust 66% of the time. – Silvio Mayolo Dec 06 '22 at 00:15
  • @Silvio, why would it be unsafe? Both answers have addressed the issue perfectly, but maybe I should have rephrased it in some other way? Thx – Yuri Astrakhan Dec 06 '22 at 01:35

4 Answers4

3

We can't move out of self, but we can replace it with an empty dummy in the meantime:

impl Foo {
    pub fn merge(&mut self, other: Self) {
        let mut tmp = Self::Many(vec![]);
        std::mem::swap(&mut tmp, self);
        *self = match (tmp, other) {
            (Self::One(a), Self::One(b)) => Self::Many(vec![a, b]),
            (Self::One(a), Self::Many(mut b)) => {
                b.insert(0, a);
                Self::Many(b)
            }
            (Self::Many(mut a), Self::One(b)) => {
                a.push(b);
                Self::Many(a)
            }
            (Self::Many(mut a), Self::Many(b)) => {
                a.extend(b);
                Self::Many(a)
            }
        };
    }
}
cafce25
  • 15,907
  • 4
  • 25
  • 31
  • Thanks @cafce25, I had to really look closely at both examples, but I think John's answer is a bit more concise. Regardless, I learnt a lot, thanks!!! – Yuri Astrakhan Dec 06 '22 at 01:14
3

What's tricky about this is that the One variant requires there to be a value present at all times. You can't take its T out, not even temporarily, because you'd have to put something else there in its place. If it were One(Option<T>) we could put a None in there, but with One(T) we can't do that.

What you can do is temporarily replace *self with an empty Many. Then generate the replacement object and overwrite the empty one so the caller doesn't see it.

impl Foo {
    pub fn merge(&mut self, other: Self) {
        let this = std::mem::replace(self, Self::Many(vec![]));
        *self = match (this, other) {
            (Self::One(a), Self::One(b)) => Self::Many(vec![a, b]),
            (Self::One(a), Self::Many(mut b)) => {
                b.insert(0, a);
                Self::Many(b)
            }
            (Self::Many(mut a), Self::One(b)) => {
                a.push(b);
                Self::Many(a)
            }
            (Self::Many(mut a), Self::Many(b)) => {
                a.extend(b);
                Self::Many(a)
            }
        };
    }
}
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
2

Just for completeness sake, I'd like to show one more way:

pub fn merge(&mut self, other: Self) {
    let self_vec = match self {
        Self::One(_) => {
            let this = std::mem::replace(self, Self::Many(vec![]));
            match (self, this) {
                (Self::Many(v), Self::One(o)) => {
                    v.push(o);
                    v
                }
                _ => unreachable!(),
            }
        }
        Self::Many(v) => v,
    };
    match other {
        Self::One(a) => self_vec.push(a),
        Self::Many(a) => self_vec.extend(a),
    };
}

(There might be some minor behavior differences in case of allocation failures, but I doubt anyone would care.)

Caesar
  • 6,733
  • 4
  • 38
  • 44
1

For completeness, another solution that avoids the temporary value:

impl Foo {
    pub fn merge(self, other: Self) -> Self {
      match (self, other) {
        (Self::One(a), Self::One(b)) => Self::Many(vec![a, b]),
        (Self::One(a), Self::Many(mut b)) => {
          b.insert(0, a);
          Self::Many(b)
        },
        (Self::Many(mut a), Self::One(b)) => {
          a.insert(0, b);
          Self::Many(a)
        },
        (Self::Many(mut a), Self::Many(b)) => {
          a.extend(b);
          Self::Many(a)
        }
      }
      
    }
}

This consumes self and returns a new Foo, so you would call it like this:

foo = foo.merge(other_foo);
harmic
  • 28,606
  • 5
  • 67
  • 91