Skip to content

#[derive_PartialEq] does not work if struct/enum carries a Boxed trait. #24047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
withoutboats opened this issue Apr 4, 2015 · 8 comments
Closed
Labels
A-type-system Area: Type system

Comments

@withoutboats
Copy link
Contributor

http://is.gd/HvndbI

#[derive(PartialEq)]
struct Foo {
    bar: Box<Baz>
}

trait Baz: PartialEq {
    fn quux(&self);
}

Results in a compiler error: cannot move out of borrowed content ... in expansion of #[derive_PartialEq]

It'd be great if the PartialEq derivation were written in such a way that this were possible.

@alexcrichton
Copy link
Member

This is currently working as intended in terms of that it doesn't compile. The PartialEq trait is not object-safe which means it's not usable through a trait object. I suspect the error message could be a little better in this case, however!

@withoutboats
Copy link
Contributor Author

The derived PartialEq is not object-safe, but in my actual code I was able to implement an object-safe PartialEq which satisfied my needs in the case of the trait that was boxed. Deriving PartialEq on the trait itself does not compile, but if the trait does implement PartialEq, wouldn't it be better for structs carrying a boxed trait to derive PartialEq? Note that if the struct is carrying the trait object as an unboxed reference, it compiles fine. This is at least an inconsistency.

Specifically in my case - and here may be Doing It Wrong - my boxed trait defines a method which a group of memberless structs all implement, expressively defining different modes of operation. I only needed PartialEq to test the mode-switching code, so the PartialEq implementation casts both self & rhs to &Any and compares their TypeIDs. Obviously this wouldn't be a proper equality check if the structs carried any data, but for my use it works.

An example of what I did (which needs nightly to compile) is below:

#![feature(core)]

use std::any::Any;

trait Foo: Any {
    fn quux(&self) -> bool;
}

impl PartialEq for Foo {
    fn eq(&self, rhs: &Foo) -> bool {
        self.get_type_id() == rhs.get_type_id()
    }
}

struct Bar;
impl Foo for Bar {
    fn quux(&self) -> bool { true }
}

struct Baz;
impl Foo for Baz {
    fn quux(&self) -> bool { false }
}

fn main() {
    println!("{}", &Bar as &Foo == &Bar as &Foo); //true
    println!("{}", &Baz as &Foo == &Baz as &Foo); //true
    println!("{}", &Bar as &Foo == &Baz as &Foo); //false
}

@murarth
Copy link
Contributor

murarth commented Feb 4, 2016

I've also run into this issue, using Rc<Trait> in an enum variant and impl PartialEq for Trait. I've employed a workaround: defining struct RcTrait(Rc<Trait>) with a non-derived PartialEq implementation. RcTrait can then be inserted into derive(PartialEq) structs or enums without triggering this issue.

@bluss
Copy link
Member

bluss commented Feb 5, 2016

Using that trait Foo from @withoutboats' code, it seems like you run into this:

struct TestStruct {
    member: Box<Foo>
}

// Compiles
impl PartialEq for TestStruct {
    fn eq(&self, rhs: &Self) -> bool {
        &self.member == &rhs.member
    }
}

//  ERROR: cannot move out of borrowed content
impl PartialEq for TestStruct {
    fn eq(&self, rhs: &Self) -> bool {
        self.member == rhs.member
    }
}

I don't see why the second should fail to compile. The == operator usually autorefs its two operands.

Full code / playpen

@bluss
Copy link
Member

bluss commented Feb 5, 2016

The code in the initial post however will never compile, so maybe edit that to clarify.

@durka
Copy link
Contributor

durka commented Mar 7, 2016

Seems like @bluss' code can be fixed by changing #[derive(PartialEq)] to put in some more &s. Any reason not to do that?

Manishearth added a commit to Manishearth/rust that referenced this issue Mar 12, 2016
cleanups and fixes for #[derive]

This contains a bunch of little cleanups and fixes to `#[derive]`. There are more extensive comments on each individual commit.

- hygiene cleanups
- use `discriminant_value` instead of variant index in `#[derive(Hash)]`
- ~~don't move out of borrowed content in `#[derive(PartialOrd, PartialEq)]`~~
- use `intrinsics::unreachable()` instead of `unreachable!()`

I don't believe there are any breaking changes in here, but I do want some more eyes on that.

Fixes rust-lang#2810 (!), I believe (we still assume that "std" or "core" is the standard library but so does the rest of rustc...).
Fixes rust-lang#21714 (cc @apasel422).
~~Fixes~~ (postponed) rust-lang#24047 (cc @withoutboats @bluss).
Fixes rust-lang#31714 (cc @alexcrichton @bluss).
Fixes rust-lang#31886 (cc @oli-obk).
bors added a commit that referenced this issue Mar 13, 2016
cleanups and fixes for #[derive]

This contains a bunch of little cleanups and fixes to `#[derive]`. There are more extensive comments on each individual commit.

- hygiene cleanups
- use `discriminant_value` instead of variant index in `#[derive(Hash)]`
- ~~don't move out of borrowed content in `#[derive(PartialOrd, PartialEq)]`~~
- use `intrinsics::unreachable()` instead of `unreachable!()`

I don't believe there are any breaking changes in here, but I do want some more eyes on that.

Fixes #2810 (!), I believe (we still assume that "std" or "core" is the standard library but so does the rest of rustc...).
Fixes #21714 (cc @apasel422).
~~Fixes~~ (postponed) #24047 (cc @withoutboats @bluss).
Fixes #31714 (cc @alexcrichton @bluss).
Fixes #31886 (cc @oli-obk).
durka added a commit to durka/rust that referenced this issue Apr 8, 2016
durka added a commit to durka/rust that referenced this issue Apr 8, 2016
…Ord)]

The derived code was doing this (assume `struct Foo(i32);`):

```
fn lt(&self, other: &Foo) -> bool {
    match *other {
        Foo(ref oi) =>
            match *self {
                Foo(ref si) => *si < *oi
            }
    }
}
```

When the struct member is a boxed trait, this can cause borrowck errors.
A solution is to change the derived code to this:

```
fn lt(&self, other: &Foo) -> bool {
    match *other {
        Foo(ref oi) =>
            match *self {
                Foo(ref si) => &*si < &*oi
            }
    }
}
```

I don't see any reason that this could break code, but I am a little nervous.
@Mark-Simulacrum
Copy link
Member

I'm not sure I quite understand what needs to be done here. The original issue implies that #[derive(PartialEq)] should work with trait objects, but this comment suggests that should never work (PartialEq isn't object-safe). The discussion then seems to shift to changing derive to make it work, which is confusing to me.

Could someone give a summary of this issue, preferably giving some concrete things that need to be done?

@withoutboats
Copy link
Contributor Author

I was pretty new to Rust when I posted this and I don't think implementing derive of PartialEq for trait objects by using their type ids is a very good idea; looks like @bluss's issue was fixed a year ago, I'm just going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system
Projects
None yet
Development

No branches or pull requests

7 participants