Skip to content
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

Catch PartialEq not being Object Safe #22040

Closed
bluss opened this issue Feb 7, 2015 · 5 comments
Closed

Catch PartialEq not being Object Safe #22040

bluss opened this issue Feb 7, 2015 · 5 comments
Assignees
Labels
A-trait-system Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Milestone

Comments

@bluss
Copy link
Member

bluss commented Feb 7, 2015

Silent corruption, from this user post

The following code compiles and runs, but produces bogus result. PartialEq and thus Expr are not object safe, and because of that Box<Expr> should have been rejected.

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

Just this is the ICE case in #18956

fn main() {
    let x: Box<PartialEq> = Box::new(1);
}
@bluss
Copy link
Member Author

bluss commented Feb 7, 2015

Comment: PartialEq can be object safe, if you substitute the Rhs assoc type with a concrete type? But Box<PartialEq> and thus Box<Expr> must be rejected.

@kmcallister kmcallister added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-nominated A-trait-system Area: Trait system and removed I-nominated labels Feb 7, 2015
@pnkfelix
Copy link
Member

1.0 beta, P-backcompat-lang.

@pnkfelix pnkfelix added this to the 1.0 beta milestone Feb 12, 2015
@nikomatsakis nikomatsakis self-assigned this Feb 12, 2015
@nikomatsakis
Copy link
Contributor

Ah, I see, the problem is actually related to defaults. That is, PartialEq is object-safe, but referencing PartialEq in an object type by itself ought to be illegal. Hmm.

@nikomatsakis
Copy link
Contributor

Never mind that last comment. I'm not 100% sure what I think is the right change here, but it is probably as simple as saying that an object-safe trait may not reference Self in the supertrait listing apart from in the Self position.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 19, 2015
The big change here is that we update the object-safety rules to prohibit references to `Self` in the supertrait listing. See rust-lang#22040 for the motivation. The other change is to handle the interaction of defaults that reference `Self` and object types (where `Self` is erased). We force users to give an explicit type in that scenario.

r? @aturon
@bluss
Copy link
Member Author

bluss commented Jun 2, 2015

Thanks for fixing this niko, this is beautiful.

error: the type parameter Rhs must be explicitly specified in an object type because its default value Self references the type Self

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
Development

No branches or pull requests

4 participants