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

Inductive cycles are treated as ambiguous in new solver, failure in old solver #50

Closed
compiler-errors opened this issue Jul 24, 2023 · 1 comment
Labels
A-coherence Having to do with regressions in `-Ztrait-solver=next-coherence`

Comments

@compiler-errors
Copy link
Member

Minimized from #113895 (interval-map):

use std::borrow::Borrow;
use std::cmp::Ordering;
use std::marker::PhantomData;

#[derive(PartialEq, Default)]
pub(crate) struct Interval<T>(PhantomData<T>);

impl<T, Q> PartialEq<Q> for Interval<T>
where
    T: Borrow<Q>,
    Q: ?Sized + PartialOrd,
{
    fn eq(&self, other: &Q) -> bool {
        true
    }
}

impl<T, Q> PartialOrd<Q> for Interval<T>
where
    T: Borrow<Q>,
    Q: ?Sized + PartialOrd,
{
    fn partial_cmp(&self, other: &Q) -> Option<Ordering> {
        None
    }
}

This code passes in the old solver, fails in the new solver.

That's because when checking if the PartialEq impls overlap (one from the Derive and one from the manual impl), we check if any of the predicates fail hold. This means we try checking Interval<?1>: PartialOrd<Interval<?1>>, which ends up being an inductive cycle with itself. Inductive cycles are treated as not holding in the old solver, since they are evaluated to EvaluatedToRecur. Meanwhile, inductive cycles in the new trait solver are treated as ambiguous.

@compiler-errors compiler-errors added the A-coherence Having to do with regressions in `-Ztrait-solver=next-coherence` label Jul 24, 2023
@lcnr
Copy link

lcnr commented Jul 24, 2023

forgot this this issue can also be used in coherence :/ anyways, already have #20, so moved the example to that issue

@lcnr lcnr closed this as completed Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coherence Having to do with regressions in `-Ztrait-solver=next-coherence`
Projects
None yet
Development

No branches or pull requests

2 participants