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

Cell is unsound around variance #11385

Closed
pcwalton opened this issue Jan 7, 2014 · 7 comments · Fixed by #11768
Closed

Cell is unsound around variance #11385

pcwalton opened this issue Jan 7, 2014 · 7 comments · Fixed by #11768
Labels
A-type-system Area: Type system I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@pcwalton
Copy link
Contributor

pcwalton commented Jan 7, 2014

The following creates a dangling pointer to an outer stack frame:

use std::cell::Cell;

struct Foo<'a> {
    x: int,
    y: Cell<Option<&'a int>>,
}

fn f() -> Foo {
    Foo {
        x: 1,
        y: Cell::new(None),
    }
}

fn g<'a>(x: &'a Foo<'a>) {
    x.y.set(Some(&x.x));
}

fn h() -> Foo {
    let x = f();
    g(&x);
    x
}

fn main() {
    let x = h();
    // kaboom
}

This manifested as a rustc segfault.

@pcwalton
Copy link
Contributor Author

pcwalton commented Jan 7, 2014

cc @nikomatsakis

@bblum
Copy link
Contributor

bblum commented Jan 7, 2014

part of #3598

@nikomatsakis
Copy link
Contributor

this is the surprising function, really:


fn h() -> Foo {
    let x = f();
    g(&x);
    x
}

Clearly this function ought not to type check. I'm not sure if this is due to some interaction with variance inference or something else, though I seem to recall another similar bug. I'll try to find some time to dig into this a bit.

@bblum
Copy link
Contributor

bblum commented Jan 8, 2014

i attempted converting this to not use cell:

struct Foo<'a> {
    x: int,
    y: Option<&'a int>,
}

fn f() -> Foo {
    Foo {
        x: 1,
        y: None,
    }
}

fn g<'a>(x: &'a mut Foo<'a>) {
    x.y = Some(&x.x);
}

fn h() -> Foo {
    let mut x = f();
    g(&mut x);
    x
}

fn main() {
    let _x = h();
}

and, as @pcwalton predicted, it wouldn't compile:

test.rs:19:7: 19:13 error: borrowed value does not live long enough
test.rs:19     g(&mut x);
                 ^~~~~~

but i don't understand why changing the mutability affects how the lifetime inference behaves.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2014

Accepted for 1.0, P-backcompat-lang.

@nikomatsakis
Copy link
Contributor

Indeed inference is incorrect yielding contravariant for 'a rather than invariant. Not sure why yet.

@nikomatsakis
Copy link
Contributor

Ah! I understand why. Interesting. Actually the inference is CORRECT, but it is being fed incorrect information. It does not know that cell mutates its value and thus is invariant. I had assumed this would be ok as we are generally invariant (right now, anyway) with respect to all type parameters, but the inference in fact computes variance correctly, and thus infers that Option and Cell are covariant with respect to their type parameter. It's just that the type system enforces a stricter standard. I remember thinking that this would be sound, but of course it's not when we haven't added proper variance notations to types like Cell. That is work which is overdue and fairly straightforward, so I'll try to do it. In some sense this is thus a dup of #10834 but I will leave it open since it reflects a specific failure we should test for.

bors added a commit that referenced this issue Feb 1, 2014
…e, r=pnkfelix

Introduce marker types for indicating variance and for opting out
of builtin bounds.

Fixes #10834.
Fixes #11385.
cc #5922.

r? @pnkfelix (since you reviewed the variance inference in the first place)
flip1995 pushed a commit to flip1995/rust that referenced this issue Sep 7, 2023
skip float_cmp check if lhs is a custom type

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: [`float_cmp`]: allow float eq comparison when lhs is a custom type that implements PartialEq<f32/f64>

If the lhs of a comparison is not float, it means there is a user implemented PartialEq, and the caller is invoking that custom version of `==`, instead of the default floating point equal comparison.

People may wrap f32 with a struct (say `MyF32`) and implement its PartialEq that will do the `is_close()` check, so that `MyF32` can be compared with either f32 or `MyF32`.
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 I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants