WIP: Unsized types

[alekratz]

See #33

This is currently a WIP implementation of support for unsized types in the GC pointers. Here's a checklist of things I've gotten versus what should probably be added before merging:

• [x] arbitrary Gc<dyn T> will compile
• [x] creation of DST by converting a Box<T>
• [x] CoerceUnsized implementation (nightly-only) for Gc<T> and compilation guards
• [x] tests for everything above

@Others is there anything else you can think of or that I'm missing from my list?

edit: removed TODOs for implementations of ToScan since it has a blanket impl now edit: removed TODO for coerce_unsized nightly feature, since it's not needed

[alekratz]

I also added a small test to the bottom of smart_ptr/mod.rs that just tries to declare a bunch of empty DSTs, but doesn't do anything with them. I was mostly using this to test that unsized types were allowed in Gc. I don't think they're necessary anymore so I can remove them (it doesn't test anything, it just fails to compile the test if unsized types aren't allowed in Gc). The only reason that it's in there is because I forgot to take it out.

[Others]

I think that test is fine. Feel free to keep it in.

PR looks pretty good to me now, pending the addition of more tests

[alekratz]

@Others currently having trouble implementing a test that uses Gc<RefCell<dyn T>>, I think because ToScan is not implemented for RefCell<dyn T>? I'm thinking it's impossible to get a &dyn Scan out of a RefCell<dyn ToScan>, since borrowing returns a guarded reference. So this is an edge case for unsized types when they may need internal mutability. What are your thoughts on how to handle this?

This is what I'm working with so far, for reference:

use std::cell::RefCell;
use shredder::*;
use once_cell::sync::Lazy;

static TEST_MUTEX: Lazy<parking_lot::Mutex<()>> = Lazy::new(|| parking_lot::Mutex::new(()));

type GcNode = Gc<RefCell<dyn LinkedListNode>>;

trait LinkedListNode: Scan + ToScan {
    fn label(&self) -> &str;
    fn next(&self) -> Option<GcNode>;
    fn set_next(&mut self, next: Option<GcNode>);
}

#[derive(Debug, Scan)]
struct BlueNode {
    next: Option<GcNode>,
}

impl LinkedListNode for BlueNode {
    fn label(&self) -> &str { "blue" }
    fn next(&self) -> Option<GcNode> { self.next.clone() }
    fn set_next(&mut self, next: Option<GcNode>) { self.next = next; }
}

#[test]
fn unlink_middle() {
    let _guard = TEST_MUTEX.lock();
    run_with_gc_cleanup(|| {
        let head_box: Box<RefCell<dyn LinkedListNode>> = Box::new(RefCell::new(BlueNode {next: None}));
        let head = Gc::from_box(head_box);
        //head.set_next(Box::new(RedNode {next: None}));
    });
}

[Others]

@alekratz Hmmm. I don't see a solution for this. We can't make a &dyn Scan pointer out of RefCell<dyn LinkedListNode>> because it isn't sized. Once the programmer has an unsized type that doesn't implement ToScan they're screwed....

I think the ToScan solution is only useful if the programmer wants to directly store a dyn T (where T: Scan + ToScan) or a dny T + ToScan. That's still an improvement, but your example shows it's flaw--it doesn't seem to work for wrapped unsized types, and it won't work for slices.

[alekratz]

@Others Do you know of any other Rust projects that have custom smart pointers that have support for unsized types without falling back to CoerceUnsized on nightly? It may be worth investigating and ~stealing~ borrowing some ideas if such a thing exists.

[alekratz]

I was only able to find one smart pointer library that supports ?Sized types after a light search of crates.io, and it appears to be using CoerceUnsized as well. This may just be the only solution for wrapped unsized types and slices like you mentioned.

Here's a link to the rust-lang issue on CoerceUnsized: https://github.com/rust-lang/rust/issues/27732 - some of the discussion seems like it's closer than some other features to be on the track to stabilization.

As far as the tests for this feature go, they still need to be written. I'm planning on keeping the current test I'm writing and just adding a conditional for nightly-only compilation, and then writing out some tests with a structure that can compile on stable as well. It's kind of annoying that we can't really make it look and act like the other smart pointers that Rust itself provides, but at this point I think it's as far as we can get until CoerceUnsized lands in stable.

[alekratz]

Still have to finish tests (and learn how to use git correctly, apparently). I'll update when I have something to show.

[Others]

It might be good to have a CI step that runs the tests on nightly with nightly features enabled. I can do that in a follow up PR (since I suspect there will be some jank involved). Just thought I'd note that here.

[alekratz]

Small update - I learned recently that the Rust compiler doesn't actually have a nightly flag that gets exported, so you have to allow users to opt-in to that via a Cargo feature. I made a new feature called "nightly-features" and I've employed it where necessary. Still working on tests both for coercion and from_box.

FWIW, I think that from_box and ToScan will be obsolete whenever the CoerceUnsized and Unsized types become stable, so keep that in mind as you move forward.

[alekratz]

@Others I've added a test for the nightly coercion stuff. I only have a single test that makes sure that dynamic types work as expected when plugged in to a Gc<T> pointer. If there are any other tests that you want to have, I'm happy to implement them.

I'm going to also start working on tests for the ToScan and from_box functions on stable, so I hope to land that soon.

[Others]

Good stuff! Thanks for pushing on this :)

[alekratz]

@Others Finally got around to writing a test for creating a Gc<dyn Trait> using from_box and can be run on stable.

[Others]

Cool! Can you fix the formatting for CI?

Will review tonight

[alekratz]

@Others Done. Let me know if there's anything else you need from me.

[Others]

Approved! Great stuff!

[Others]

Oops forgot to remove the WIP tag when I merged! Still, it's in master now!