Comparing boxed traits (possible bug?)


#1

While writing a toy lisp interpreter I noticed a strange issue where my boxed traits were not equal. After writing my own PartialEq trait implementation it looks like the rhs parameter appears to be corrupt/uninitialized.

Here is a trimmed down playpenified version: http://is.gd/Z8i7qk

use std::fmt::Debug;

trait Expr: Debug + PartialEq { 
    fn print_element_count(&self);
}

//#[derive(PartialEq)]
#[derive(Debug)]
struct SExpr<'x> {
    elements: Vec<Box<Expr+ 'x>>,
}

impl<'x> PartialEq for SExpr<'x> {
    fn eq(&self, other:&SExpr<'x>) -> bool {
        println!("L1: {} L2: {}", self.elements.len(), other.elements.len());
        let result = self.elements.len() == other.elements.len();

        println!("Got compare {}", result);
        return result;
    }
}
    
impl <'x> SExpr<'x> {
    fn new() -> SExpr<'x> { return SExpr{elements: Vec::new(),}; }
}

impl <'x> Expr for SExpr<'x> {
    fn print_element_count(&self) {
        println!("element count: {}", self.elements.len());
    }
}

fn main() {
    let a: Box<Expr> = Box::new(SExpr::new());
    let b: Box<Expr> = Box::new(SExpr::new());
    
    a.print_element_count();
    b.print_element_count();
    
    assert_eq!(a , b);
}

Outputs:

element count: 0
element count: 0
L1: 0 L2: 139885991465824
Got compare false
thread '<main>' panicked at 'assertion failed: `(left == right) && (right == left)` (left: `SExpr { elements: [] }`, right: `SExpr { elements: [] }`)', <anon>:40
playpen: application terminated with error code 101

It looks like the len() of the elements array is returning an initialized value. Is this a bug or I am just going about this in a terrible way?


Usecases for inheritance
#2

PartialEq shouldn’t be an “object-safe trait”, so it should error already at the conversion to Box<Expr>! Definitely a bug either way.


#4

I think PartialEq is not object-safe, because it also takes, apart from the &self parameter, a second other: &Self parameter.


#5

Interesting I guess I will go make sure this bug is filed if it isn’t already.

Also repros with just borrowed base traits (no boxing required): http://is.gd/Kw5W9f I guess I will need to dig into object safety since you would think a reference could provide that. At least for the &self reference it looks like it is working.

Bug aside: How do you compare polymorphic things with each other in rust? Is this not possible? Not the end of the world since I can just switch this to an enum I guess. I suppose another option could be building my own trait that is capable of keeping my objects safe and do the comparison there.


#6

The bug is that rustc has failed to stop you in time from writing unsound code. It generally tries to do that, and it will do that here too once the “object safety” bug is fixed.

For your type you defined how to compare for equality against its own type, but then expanded that via an object type to compare “all types that can compare to themselves” into comparing with each other. Of course there is no sound way to compare T and U just from the implementations of T: PartialEq<T> and U: PartialEq<U> (T knows to compare with T, and U with U).

How to do it in general? Either you have to use a custom method in the trait, or you define PartialEq on the trait object types themselves. In both cases, you’ll have to access only the trait methods when doing the comparison, you have no access to the internals of the “other” value.


#7

Didn’t find any issue reported, so I posted it here


#8

Thanks! I never got around to posting the bug. Been trying to get a version of this working and hit a couple ICEs along the way (turns out the current build of rust really hates recursive traits).


#9

I think I have a version this is close to what you were describing http://is.gd/hikaWC but it looks like it hits https://github.com/rust-lang/rust/issues/21756

Probably more of the compiler trying to tell me I am being a fool but exploding while doing so.


#10

Using Box<SExpr> is ok, and it compiles & compares successfully. Expr needs to remove its inheritance from PartialEq, because allowing that is a bug: inheriting PartialEq<Rhs=Self> precludes using Expr as a trait object (and thus SExpr, inheriting it).