nikis05 / derive-visitor

MIT License
20 stars 6 forks source link

breakage due to a bugfix in rustc #16

Closed lcnr closed 2 months ago

lcnr commented 3 months ago

Your crate will unfortunately stop compiling once https://github.com/rust-lang/rust/pull/121848 is merged. It unintentionally relies on an unsound bug of the type system which we are now fixing: https://github.com/rust-lang/rust/issues/114061.

https://github.com/nikis05/derive-visitor/blob/39dc342fb1e9ffcdec3e25c00d46a146418b418f/derive-visitor/src/lib.rs#L538-L544

This can overlap with the following impl

https://github.com/nikis05/derive-visitor/blob/39dc342fb1e9ffcdec3e25c00d46a146418b418f/derive-visitor/src/lib.rs#L566-L568

causing Box<Local> to implement Drive using two separate overlapping impls in the following example:

use std::iter;
struct Local;
impl<'a> IntoIterator for &'a Box<Local> {
    type IntoIter = iter::Once<&'a ()>;
    type Item = &'a ();
    fn into_iter(self) -> Self::IntoIter {
        iter::once(&())
    }
}

impl derive_visitor::Drive for Local {
    fn drive<V>(&self, _: &mut V) {}
}

fn impls_drive<T: derive_visitor::Drive>() {}

fn main() {
    impls_drive::<Box<Local>>();
}

The underlying issue is that, during the coherence overlap check, we consider for<'a> <&'a Box<_> as IntoIterator>::Item: DerefAndDrive to never apply, even though as shown above, it is possible to writean impl so that it does.

I think that there is unfortunately no way to fix the new error except by remove the blanket impl. I apologize for the this.

lovasoa commented 3 months ago

Do you mean that this library will be forever broken without the possibility to implement a fix ?

lcnr commented 3 months ago

The impl as is cannot be supported given that it overlaps with other impls. I expect there to be a way to rewrite this to work mostly the same though.

lovasoa commented 3 months ago

Maybe you could open a PR, then?

nikis05 commented 3 months ago

@lcnr thanks for reporting this.

I think we should go back to macro-based impls for concrete collection types. @lovasoa @badicsalex thoughts?