Doc review!

[CAD97]

I try to write great documentation, but I need others' eyes to make sure it's good. Check out the master documentation and put any feedback in this thread. (Actionable issues may of course still get their own issues.)

[danielhenrymantilla]

Neat crate! I like it!! 🙂

Regarding https://cad97.github.io/pointer-utils/erasable/trait.ErasablePtr.html#safety, I think adding a counter example there would be helpful. It's kind of the same issue that the pin docs had for some time: it is hard to visualize the invariants without counterexamples for each missing invariant 😅

[CAD97]

The requirements of ErasablePtr are hard to create counterexamples for, because they primarily are protecting against degenerate cases 😅 I'm actually failing to come up with a "useful" example that would actually be able to roundtrip through an erased pointer and fail to uphold the requirements, because failing to do so is such a degenerate case.

Two simple degenerate cases I can actually come up with are shown in #26.

[Ralith]

I'd really love to see a detailed rationale for slice-dst's soundness. The existing documentation handwaves it, and the implementation appears to have hardcoded assumptions about the layout rustc uses for fat pointers. I didn't see any comments or documentation justifying these assumptions, and as far as I can tell the layout in question is explicitly not guaranteed to be stable.

[CAD97]

implementation appears to have hardcoded assumptions about the layout rustc uses for fat pointers

This isn't quite right.

I make no assumption of how the pointer itself is laid out. ptr::slice_from_raw_parts_mut is a standard library way to construct a fat pointer from a thin pointer and the slice length (fn<T>(*mut T, usize) -> *mut [T]).

After that, it's just the heap structure that we have to make line up. Box is guaranteed to use the global allocator, and for manual alloc/dealloc using Rust's global allocator to be allowed. Array layout and slice layout are specified to be the obvious (as provided by Layout::array). #[repr(C)] is specified layout, and now we've found specification for the entire heap layout of the type we're allocating.

If you've got a suggestion for where to elaborate on these details, I'd be happy to add some notes. It might help to note that everything in layout_polyfill.rs is a polyfill for feature(alloc_layout_manipulation), which stabilized in Rust 1.44 (and I'm doing a patch to use it on Rust ^1.44).

[Ralith]

If you've got a suggestion for where to elaborate on these details, I'd be happy to add some notes.

The code that makes me particularly anxious is in StrWithHeader::new and the analogous, but more complex, case for slices. Reviewing more carefully, it looks like this is populating the target of the fat pointer rather than the fat pointer itself--but then why is there a length field, since that's already represented in the metadata?

More broadly, it would be nice to have an discussion of the strategy in the module docs, in considerably more detail than "we can manually allocate the Box" and "rust handles it".

#[repr(C)] is specified layout

I can't find any use of #[repr(C)] in slice-dst whatsoever.

[CAD97]

#[repr(C)]

SliceWithHeader and StrWithHeader are #[repr(C)]; this is what allows us to know their layout and write them into a pointee-uninitialized pointer.

but then why is there a length field, since that's already represented in the metadata?

Ah, yes, that might be confusing. The length is redundantly stored inline as well such that the types are Erasable -- pointers to them can be roundtripped through thin untyped pointers. If I could rewind time, I would try harder to have this functionality be another layer around [Str|Slice]WithHeader, probably, but I've yet to find a truly satisfactory design (and it would require a breaking version bump at this point anyway).

The fact that these types always carry the slice length inline is one at least partially of legacy; the predecessor crate of thin-dst was primarily about supporting thin pointers to slice-based DSTs, and triomphe, the real prior art, also did it this way.

[Ralith]

SliceWithHeader and StrWithHeader are #[repr(C)]; this is what allows us to know their layout and write them into a pointee-uninitialized pointer.

Whoops, indeed they are. Somehow I skimmed that on manual review, and I guess rg parsed it as regex syntax...

The length is redundantly stored inline as well such that the types are Erasable -- pointers to them can be roundtripped through thin untyped pointers.

Would it make sense to feature-guard them, at least? That would both reduce overhead and make their function clearer.

[CAD97]

Would it make sense to feature-guard them, at least?

The problem with just doing that is that then all members of the structs are pub. This makes re-adding the not-pub field a breaking change.

I've got the WithHeader<Header, str>/WithHeader<Header, [T]>/WithHeader<WithLength<Header>, [T]>> design stuck in my head again, I'll probably spend some time seeing if I can make that work and deprecate the old names.

[Ralith]

The problem with just doing that is that then all members of the structs are pub

#[non_exhaustive]? A private () field?

I'll probably spend some time seeing if I can make that work and deprecate the old names.

That works too; good luck!

[CAD97]

#63 adds some big-picture documentation to slice-dst.

[jplatte]

I think you mean #64 ;)