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

Ambiguity in assemble_candidates_after_normalizing_self_ty causes coherence error #52

Closed
compiler-errors opened this issue Jul 24, 2023 · 2 comments
Labels
A-coherence Having to do with regressions in `-Ztrait-solver=next-coherence` S-breaking-change

Comments

@compiler-errors
Copy link
Member

compiler-errors commented Jul 24, 2023

This coherence failure...

pub(crate) trait Drive {}

trait DerefAndDrive {}

impl<T> Drive for T
where
    T: 'static,
    for<'a> &'a T: IntoIterator,
    for<'a> <&'a T as IntoIterator>::Item: DerefAndDrive,
{
}

impl Drive for () {}

fn main() {}

... is because the goal <&'a () as IntoIterator>::Item: DerefAndDrive is considered ambiguous when unifying the two impls above. That's because we try normalizing the self type of the goal, and &'a (): IntoIterator is treated as unknowable.

I think this is problematic, because even if we were to add an upstream implementation to be able to normalize <&'a () as IntoIterator>::Item, it couldn't implement DerefAndDrive since that's a trait local to the crate.

@lcnr
Copy link
Contributor

lcnr commented Jul 25, 2023

I don't think we want to add any reasoning like this 🤔 at least if we can avoid it. It feels quite subtle to get right and complicates our rules.

The general pattern here is allowed because of an unsoundness in the old solver, see rust-lang/rust#114061. If we care about not breaking the affected crate we should instead add a negative impl to core and rely on negative coherence to get for<'a> &'a T: IntoIterator to fail.

@lcnr
Copy link
Contributor

lcnr commented Apr 29, 2024

opened nikis05/derive-visitor#16, closing

@lcnr lcnr closed this as completed Apr 29, 2024
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` S-breaking-change
Projects
None yet
Development

No branches or pull requests

2 participants