matthieu-m / static-rc

Compile-time reference counting
Apache License 2.0
428 stars 13 forks source link

Consider to design `StaticRc` as a linear type? #7

Open frank-king opened 3 years ago

frank-king commented 3 years ago

In current implementation, a StaticRc<T, NUM, DEN> where NUM < DEN, it will do nothing on Drop. It can probably lead to memory leak if a "full" pointer is split up to two, and the two pointers are dropped silently.

How about to design the StaticRc<T, NUM, DEN> to be a linear type (i.e., it must be use and only use once). To say an object is fully used in the static reference counting system means: it is created as a full pointer like Box, and might be split up to several sub-pointers that shares the ownity, but must finally joined into a full pointer again, then the pointer is automatically dropped.

So the main implementation idea is: if I drop a non-full pointer, there will be a compile-time error because it is the object is not fully used.

matthieu-m commented 3 years ago

StaticRc should, indeed, be a linear type.

Unfortunately, to the best of my knowledge there is no way to implement linear types in Rust, and therefore as a "placeholder" there is a debug_assert! in the Drop implementation to notify the user if NUM < DEN.

In the absence of specialization and negative trait implementation, it feels to me like the only way to produce a compilation error would be to provoke a monomorphization error if NUM < DEN.

Did you have another idea? Or do you know how to produce a (nice) monomorphization error?

frank-king commented 3 years ago

I have tried to introduce a static assertion in the Drop::drop method, using the static_rc::const_assert! macro.

struct Linear;

impl Drop for Linear {
    fn drop(&mut self) {
        const_assert!(false)
    }
}

fn main() {
   'a: { let _linear = Linear; } // Should not compile
   'b: { let _linear = std::mem::ManuallyDrop(Linear); } // Should compile
}

Unfortunately, it doesn't work because the const_assert! will yield a unconditional compile-time failure in both 'a and 'b.

Currently, as you say, it might only be possible to use a debug_assert! to forbid the type to be dropped and checks in runtime.

5225225 commented 2 years ago

From some brief tests using the dont_panic crate (last touched 5 years ago but still seems to work fine), the tests compile when running in release mode, except for the doctests (which don't seem to compile in release even though you have --release)

impl<T: ?Sized, const NUM: usize, const DEN: usize> Drop for StaticRc<T, NUM, DEN> {
    #[inline(always)]
    fn drop(&mut self) {
        if NUM == DEN {
            //  Safety:
            //  -   Ratio = 1, hence full ownership.
            //  -   `self.pointer` was allocated by Box.
            unsafe { Box::from_raw(self.pointer.as_ptr()) };
        } else {
            dont_panic::dont_panic!("Drop of incomplete StaticRc")
        }
    }
}

And yeah, it doesn't work at all without optimizations. So you'd want to gate this on "has release optimizations", which doesn't seem too hard. Use an assertion if optimisations are disabled, use this if they are enabled and a feature is set?

matthieu-m commented 2 years ago

@5225225 I'd love to have a compile-time error, however shouldn't we ideally aim for an error in Debug, rather than Release?

I suppose having debug_assert! in Debug and dont_panic! in Release is a possibility, but it is not clear to me how much benefit the latter would yield.

5225225 commented 2 years ago

Yes, but you need to use optimisations in order to use dont_panic. So I'd expect users of this crate to put a release build in their CI in addition to their debug builds (we may also want to put it behind a cfg for when the compiler can't prove it?)