hacspec / hax

A Rust verification tool
https://hacspec.org/blog
Apache License 2.0
195 stars 20 forks source link

Cyclic dependency introduced by bundling with `let open` #1070

Open maximebuyse opened 2 weeks ago

maximebuyse commented 2 weeks ago
pub mod a {
    #[derive(Debug)]
    pub struct S {}
    pub struct Sa(super::b::Sb);
    impl std::fmt::Debug for Sa {
        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
            write!(f, "{:?}", self.0)
        }
    }

}

pub mod b {
    pub struct Sb(super::a::S);
    impl std::fmt::Debug for Sb {
        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
            write!(f, "{:?}", self.0)
        }
    }
}

Open this code snippet in the playground

In the situation above, the recursive bundle contains a let open for one of the original modules creating a cycle.

W95Psp commented 1 week ago

Can this be related to https://github.com/hacspec/hax/issues/1068?

maximebuyse commented 1 week ago

Can this be related to #1068?

I need to investigate more. This one is about impl_expr, I think it is more a problem of not renaming things. The other one is about actually moving definitions to the bundle so I think it is a bit different.

maximebuyse commented 1 week ago

I found a way to minimize even more:

pub mod a {
    #[derive(Debug)]
    pub struct S {}
   fn f(_x:super::b::Sb) {}

}

pub mod b {
    pub struct Sb(super::a::S);
    impl std::fmt::Debug for Sb {
        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
            write!(f, "{:?}", self.0)
        }
    }
}

Open this code snippet in the playground

The problem seems to come from the impl_expr for the derive(Debug) for S

maximebuyse commented 1 week ago

So the problem is that we filter out automatically generated items in import_thir, including derived trait impls. Some impl_expr still refer to them but as they don't exist in the engine we don't move them to the bundle, so we don't rename the references to them, thus creating a fake implicit dependency.

maximebuyse commented 1 week ago

After discussing this, we should:

maximebuyse commented 1 week ago

Related issue: https://github.com/hacspec/hax/issues/108

maximebuyse commented 5 days ago

I am testing my implementation of this. One problem in the case of Debug is that Debug and Formatter both belong to core::fmt and have a method fmt which means we get https://github.com/hacspec/hax/issues/61. We could rename the method in one of them and also do the same renaming when we translate an implementation. But this is a bit of a hack...

maximebuyse commented 5 days ago

I am testing my implementation of this. One problem in the case of Debug is that Debug and Formatter both belong to core::fmt and have a method fmt which means we get #61. We could rename the method in one of them and also do the same renaming when we translate an implementation. But this is a bit of a hack...

Apart from that the current implementation works but with the false precondition for panic it will not be usable for proofs. We can try to find an alternative using vals (but it might depend if we want to extract interfaces only, or implementation too.

For some traits like Clone we have a custom definition without pre/post conditions which makes the generated code for impls incompatible with the trait definition. We could filter out impls of Clone but that would leave the potential problem of creating cyclic dependencies with impl_expr.