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

Consider to design StaticRc as a linear type? #7

Open
frank-king opened this issue Sep 3, 2021 · 5 comments
Open

Consider to design StaticRc as a linear type? #7

frank-king opened this issue Sep 3, 2021 · 5 comments

Comments

@frank-king
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link

5225225 commented May 21, 2022

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
Copy link
Owner

@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
Copy link

5225225 commented Jun 2, 2022

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?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants