Pre-pre-RFC: `NoDrop` marker trait

It's not clear to me why one would implement Drop against a !ScopeDrop type, it inhibits destructuring the type (let Self { _linear_marker, data } = self;), which is the most expected approach for proper clean-up. In the proposal, it is possible to implement Drop for such a type, but the only way for it to be invoked is via panic-based stack-unwinding. What I suggested for the escape hatch was to allow a user to wrap the type, implement a Consume trait against the wrapper type (whose consume function takes self by move), and the Drop::drop method then forwards the wrapped type to Consume::consume. One doesn't define Drop::drop for a linear type, but for an affine wrapper for a linear type, and it requires unsafe code to make that work.

This is the larger concern, for me. I can imagine addressing it via changing the signature of the safe variant:

fn forget<T: ScopeDrop>(a: T);
unsafe fn forget_unsafe<T: ?ScopeDrop>(a: T);

I am not sure if this is a good idea or not -- it could be a good idea if linear types aren't vulnerable to leakpocalypse concerns. Here's a leakpocalypse example (from here):

fn forget<T>(val: T) {
    use std::cell::RefCell;
    use std::rc::Rc;
    struct Foo<T>(T, RefCell<Option<Rc<Foo<T>>>>);
    let x = Rc::new(Foo(val, RefCell::new(None)));
    *x.1.borrow_mut() = Some(x.clone());
}

if T is linear, then Rc<T> is also linear, and x would be prevented from falling out of scope. But I'd want to prove we can't create forget safely with linear types before suggesting we can bifurcate forget this way.

But that unsafe code can't assume Drop::drop will be called, so any such unsafe code would be unsound.

Similar solutions were already proposed in the past, the problem is that T: ScopeDrop would need to be an implied bound and that means types not implementing ScopeDrop would become incompatible with the current ecosystem.

1 Like

Proposed name of trait is confusing. I thought that you propose some marker trait which means that object doesn't require to be dropped (e.g. POD values like u8).

I feel we must be talking past each other... Drop::drop isn't expected to be defined when Consume::consume is used for the escape hatch. Here's how I think it works:

// a linear type
struct MyLinear {
    // made linear by this marker trait
    _linear_marker: PhantomLinear,
    value: i32,
}
impl MyLinear {
    // gets cleaned up here:
    fn clean_up(self) {
        let Self { _linear_marker, value } = self;
        _linear_marker.consume();
    }
}
// we don't expect `Drop` to be defined for `MyLinear`: it's possible
// to do so, but it inhibits the `let Self { _linear_marker, value } = self`
// above, used for destructuring clean-up.

// using the escape hatch is awkward, you need to define two
// classes, sigh. this is the first, and it implements `Consume`.
// users aren't expected to directly declare this.
struct AffineWrapperImpl(MyLinear);
impl Consume for AffineWrapperImpl {
    fn consume(self) {
        self.0.clean_up();
    }
}
// this is what users are expected to use. In the blog post,
// this is `type AffineWrapper = ReScopeDrop<AffineWrapperImpl>;`,
// but I'm removing the generic stuff to make this more concrete.
struct AffineWrapper(ManuallyDrop<AffineWrapperImpl>);
impl Drop for AffineWrapper {
    fn drop(&mut self) {
        let impl: AffineWrapperImpl = unsafe { self.0.take() };
        impl.consume();
    }
}

Drop::drop is defined on AffineWrapper, but should not be defined on AffineWrapperImpl, or on MyLinear.

Yes, that's true. This isn't purely cost, this is also benefit (really, I think it's the point): you don't want to just use affine-oriented generic code against linear types, it's pretty unlikely to work correctly. Addressing this still suggests a lot of ecosystem churn... I believe the cost of this churn can be significantly reduced with automated refactoring tools (it's easy to analyze code to determine if it works with linear constraints), but even so, this might be a showstopping problem.

It's still not clear to me why AffineWrapper is sound. You can use it to wrap an instance of MyLinear and then leak it, thus leaking an instance of a linear type.

What prevents me from doing this:

fn forget_linear(linear: MyLinear) {
    let wrapperimpl = AffineWrapperImpl(linear);
    let wrapper = AffineWrapper(std::mem::ManuallyDrop::new(wrapperimpl));
    std::mem::forget(wrapper);
}

The only way I can see this working is if creating AffineWrapper was unsafe, but that makes the escape hatch not practical at all.

But I would also want to reuse the generic code that currently works for both.

I'm skeptical this is possible in the general case, in particular if unsafe code and raw pointers are involved.

But even then any generic code compatible with linear types would be cluttered with T: ?ScopeDrop.

Thank you for your patience, I see your point now. I'll have to dwell on this... Certainly for now, I agree, I don't see a way out of this issue while keeping the escape hatch, and the escape hatch seems important for the ecosystem.

In most cases, it seems easy: If the compiler doesn't need to insert drop glue in any possible monomorphization (except for unwind), and the user isn't calling code that requires T: ScopeDrop, then you allow T: ?ScopeDrop.

What prevents mem::forget on a NoDrop type to be a compile time error?

Specialcasing std::mem::forget is not a solution because everyone can write their own and it's considered safe. Changing this would be a breaking change.

1 Like

I don't even think this is possible.

Folks can write their own forget for affine types, it's not obvious (to me) that you can write your own for linear types. The proof-of-concept safe forget looked like:

fn safe_forget<T>(data: T) {
    use std::rc::Rc;
    use std::cell::RefCell;

    struct Leak<T> {
        cycle: RefCell<Option<Rc<Rc<Leak<T>>>>>,
        data: T,
    }

    let e = Rc::new(Leak {
        cycle: RefCell::new(None),
        data: data,
    });
    *e.cycle.borrow_mut() = Some(Rc::new(e.clone())); // Create a cycle
}

but if T: !ScopeDrop (using my terminology), then this should fail to compile, as Rc<T: !ScopeDrop> is also !ScopeDrop, and thus e falling out of scope at the bottom would be a compilation error.

Maybe I read that question way too literally, but it only asked what prevents std::mem::forget on NoDrop type to be a compile time error, not other functions. Of course we already discussed ways to make other functions like safe_forget unable to be called with NoDrop types, but that's different than just making std::mem::forget on NoDrop type a compile time error, and of course the reasons why this doesn't work don't apply to the rest of the discussion.

If ScopeDrop acts like Sized (type parameter T is assumed to include ScopeDrop, and if you want to allow others, then you need ?ScopeDrop), then std::mem::forget (or even core::intrinsics::forget) wouldn't be defined for T: !ScopeDrop. (I apologize for switching terminology, but ScopeDrop is easier for me to reason about than NoDrop.)

1 Like

Btw

  1. Linear types (NoDrop) could be useful for the async Drop case (i.e. implicit drop is forbidden, a user have to manually call foo.close().await) in order to get rid of some value.
  2. Linear types could be very useful for things like StaticRc
1 Like

I wrote up a concrete proposal based on @aidancully 's Communicating With Code: Less Painful Linear Types, in the format of an RFC. Take a look, and if you like it I'll open a PR in the rfcs repo.

RFC: ScopeDrop

Note that the lang team is incredibly skeptical of any new ?Trait bounds. See how even pinning didn't get one.

My first impression after skimming the RFC is that it makes this "non-breaking" technically, but at the cost of making these types non-usable in basically everything that already exists. You can't even return them in a Result or make a NonNull pointer to them.

So I'd love to see more about what this might look like as a lint (like you address in passing) -- especially since you mention this still couldn't be relied upon for soundness.

3 Likes

To add on some prior art and an example of catching practical problems, recently someone wrote about how they used a pattern similar to NoDrop in the Vale language and it was genuinely useful in their coding efforts.

As long as it's opt-in, a tool like NoDrop seems like it would be quite a useful tool for API designers. It should not be relied on for enforcing memory safety, but it is more for enforcing logical correctness. From a memory safety perspective, "linear" NoDrop types should still expect that it can be passed into std::mem::drop or std::mem::forget.

The correctness of the code would be improved in the standard, non-panicking situation. Logical correctness is less important in a panic situation, and I think adding NoDrop wouldn't worsen correctness in panic situations.

2 Likes

Maybe a lint marker like #[no_implicit_drop] for a type could be sufficient and easier to implement? It's a warning by default but can be elevated to a compiler error via #[deny(no_implicit_drop)]. As a comparison point, #[must_use] is easily worked around, but is a code smell and can be a sign of incorrect code.

let _ = foo_returns_result();

#[no_implicit_drop] could behave like so: if when compiling a function, drop code is automatically inserted, then generate the warning. Generating drop code for panic unwinding is fine. Manual use of drop will not generate the warning. A user could just drop the value manually, but that would be a code smell, similar to the ignored #[must_use] result.

#[no_implicit_drop]
struct NoImplicitDrop {}

impl NoImplicitDrop {
    pub fn new() -> self {
        Self{}
    }

    pub fn consume(self) {
        // need to manually type this to suppress the warning
        std::mem::drop(self)
    }
}

fn proper_usage() {
    let v = NoImplicitDrop::new();
    v.consume();
}

// catch this in code review, manually calling drop here looks weird
fn code_smell() {
    let v = NoImplicitDrop::new();
    std::mem::drop(v);
}

// can turn into compiler error with #[deny(no_implicit_drop)]
fn warning() {
    let v = NoImplicitDrop::new();
}

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.