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

Unexpected lifetime issue when comparing Option<&i32> #42966

Closed
CasualX opened this issue Jun 29, 2017 · 9 comments
Closed

Unexpected lifetime issue when comparing Option<&i32> #42966

CasualX opened this issue Jun 29, 2017 · 9 comments
Labels
A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@CasualX
Copy link

CasualX commented Jun 29, 2017

I tried this code:

fn check(a: Option<&i32>, b: &i32) -> bool {
    a == Some(b)
}

I expected to see this happen: Code compiles and works as expected.

Switching the equality around like this compiles successfully and works as expected:

fn check(a: Option<&i32>, b: &i32) -> bool {
    Some(b) == a
}

Alternatively explicitly forcing both references to a shared lifetime works too:

fn check<'c>(a: Option<&'c i32>, b: &'c i32) -> bool {
    a == Some(b)
}

Instead, this happened: error[E0495]: cannot infer an appropriate lifetime for automatic coercion due to conflicting requirements

Meta

https://play.rust-lang.org/?gist=9bd7c205c280df1ea37bb49cd3ecca25&version=stable&backtrace=0

@tirr-c
Copy link
Contributor

tirr-c commented Jun 29, 2017

The compiler says that this code, with explicit lifetime parameters, has a type mismatch between Option<&'a i32> and Option<&'b i32>:

fn check<'a, 'b>(a: Option<&'a i32>, b: &'b i32) -> bool {
    let tmp: Option<&'b i32> = Some(b);
    a == tmp //< mismatched types
}

It says 'b should outlive 'a, and the bound is not needed when we compare two simple references. Kinda confusing; anyway, as said, I added a lifetime bound.

fn check<'a, 'b>(a: Option<&'a i32>, b: &'b i32) -> bool where 'b: 'a {
    let tmp: Option<&'b i32> = Some(b);
    a == tmp
}

And it compiles well.

What's strange is, PartialEq here cannot be symmetric; following code doesn't compile.

fn check<'a, 'b>(a: Option<&'a i32>, b: &'b i32) -> bool where 'b: 'a {
    let tmp: Option<&'b i32> = Some(b);
    let eq1 = a == tmp;
    let eq2 = tmp == a; //< mismatched types
    assert_eq!(eq1, eq2);
    eq1
}

This is definitely wrong.

@tirr-c
Copy link
Contributor

tirr-c commented Jun 29, 2017

More testcases. This works well:

fn should_equal<'a, 'b>(a: Option<&'a usize>, b: &'b usize) {
    let b = Some(b);
    assert_eq!(a, b);
    assert_eq!(b, a);
}

However, this fails compiling:

fn should_equal<'a, 'b>(a: Option<&'a usize>, b: &'b usize) {
    let b = Some(b);
    assert!(a == b);
    assert!(b == a);
}

@oli-obk
Copy link
Contributor

oli-obk commented Jun 30, 2017

The PartialEq impl for Option is derived. This sounds like a bug with derive

@fintelia
Copy link
Contributor

fintelia commented Jul 3, 2017

This is the code generated by derive:

#[automatically_derived]
#[allow(unused_qualifications)]
#[stable(feature = "rust1", since = "1.0.0")]
impl <T: ::std::cmp::PartialEq> ::std::cmp::PartialEq for Option<T> {
    #[inline]
    fn eq(&self, __arg_0: &Option<T>) -> bool {
        {
            let __self_vi =
                unsafe { ::std::intrinsics::discriminant_value(&*self) } as
                    isize;
            let __arg_1_vi =
                unsafe { ::std::intrinsics::discriminant_value(&*__arg_0) } as
                    isize;
            if true && __self_vi == __arg_1_vi {
                match (&*self, &*__arg_0) {
                    (&Option::Some(ref __self_0),
                     &Option::Some(ref __arg_1_0)) =>
                    true && (*__self_0) == (*__arg_1_0),
                    _ => true,
                }
            } else { false }
        }
    }
    #[inline]
    fn ne(&self, __arg_0: &Option<T>) -> bool {
        {
            let __self_vi =
                unsafe { ::std::intrinsics::discriminant_value(&*self) } as
                    isize;
            let __arg_1_vi =
                unsafe { ::std::intrinsics::discriminant_value(&*__arg_0) } as
                    isize;
            if true && __self_vi == __arg_1_vi {
                match (&*self, &*__arg_0) {
                    (&Option::Some(ref __self_0),
                     &Option::Some(ref __arg_1_0)) =>
                    false || (*__self_0) != (*__arg_1_0),
                    _ => false,
                }
            } else { true }
        }
    }
}

@fintelia
Copy link
Contributor

fintelia commented Jul 3, 2017

And a "simpler" case that generates the same error:

pub struct Foo<'a>(&'a usize);

impl<'a> ::std::cmp::PartialEq for Foo<'a> {
    fn eq(&self, _: &Foo<'a>) -> bool { true  }
    fn ne(&self, _: &Foo<'a>) -> bool { false  }
}

fn should_equal<'a, 'b>(a: Foo<'a>, b: &'b usize) {
    assert!(a == Foo(b));
}

@PlasmaPower
Copy link
Contributor

This appears to be a problem with the equal operator, not the PartialEq implementation or trait, as this works fine: https://play.rust-lang.org/?gist=32afb56e08f6d275711698f0a4fc7f61&version=stable&backtrace=0

fn check(a: Option<&i32>, b: &i32) -> bool {
    a.eq(&Some(b))
}

@Mark-Simulacrum Mark-Simulacrum added the A-lifetimes Area: Lifetimes / regions label Jul 19, 2017
@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 28, 2017
@CasualX
Copy link
Author

CasualX commented Sep 10, 2018

This issue looks resolved? The error no longer occurs when I try my example in the playground.

However I've no idea when, why or what caused the issue. Can anyone confirm as fixed?

@matthewjasper
Copy link
Contributor

Fix was probably #45435

@CasualX
Copy link
Author

CasualX commented Jan 30, 2019

that sounds plausible, I'll close this issue then

@CasualX CasualX closed this as completed Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants