Seprate HeaderSliceWithLength into checked and unchecked variants

[Manishearth]

Fixes #107

The breakage is limited to with_mut(), which now returns a HeaderSliceWithLengthChecked which has an immutable header.

Not fully happy with the resultant API, worth cleaning up in the next major release.

[Manishearth]

r? @steffahn if you have time

[steffahn]

Fixes #107

This addresses only half of #107. Switching out the whole Arcs is still a problem.

The with_mut callback can add a check for this. (Pointer equality, I guess? Actually, I suppose simply unconditionally writing back the new pointer could be okay.)

[steffahn]

Probably only with_arc_mut should change, this is breaking old API.

[steffahn]

I’m not really happy with the naming, haven’t thought of better alternatives yet.

The problem is that now there’s Arc::from_thin and Arc::checked_header_from_thin, and the latter just sounds more heavy, like there’s additional checking going on, even though the exact opposite is the case (you get something that needs less checking in order to convert back).

---

Actually, now I have an idea: how about the word becomes “protected”?

E.g. HeaderSliceWithLengthProtected instead of HeaderSliceWithLengthChecked. (And the HeaderSliceWithLengthUnchecked type synonym isn’t needed at all.)

Then we can have Arc::from_thin and Arc::protected_from_thin. That sounds less like additional “checking” is going on.

And we can have ThinArc::with_arc and ThinArc::with_protected_arc (the latter can also serve to be used internally an implementation that replaces the unsafe check-less clone from #76). We could even consider to keep with_arc_mut [with sufficient checks, i.e. under documented threat of abort] and add with_protected_arc_mut next to it, making the naming more consistent.

---

Or (coming back to naming) another idea is HeaderSliceWithConstLen, and thus Arc::const_len_from_thin, ThinArc::with_const_len_arc and ThinArc::with_const_len_arc_mut.

[Manishearth]

Yeah I'm not happy with the naming either, this is just the naming that's stable. The unchecked variant should be more verbose.

[Manishearth]

Protected seems fine for now. I don't like the implication of const here with the const naming.

[steffahn]

Okay, finally starting another round of review. I’m wondering if with_protected_arc should be made public, I think it might be weird if the “more useful” (with stronger guarantees) protected version is only exposed to users of the mutable API.

[steffahn]

I wonder if HeaderSliceWithLengthProtected should just contain a slice directly, instead of T: ?Sized. I feel the invariant is hard(-ish) to formulate anyways. In the presence of unsizing coercions, a [T; N] in a HeaderSliceWithLengthProtected would also need to have correct length information.

Other types kind-of have no rules… except str is mentioned in the current docs.

Ultimately, HeaderSliceWithLengthProtected is a tool for a sound API of ThinArc and ThinArc doesn't support anything but [T]. Even the str case seems relatively irrelevant, because ThinArc doesn't support it.

[steffahn]

I wonder if HeaderSliceWithLengthProtected should just contain a slice directly, instead of T: ?Sized.

@Manishearth I've tried this out myself, so you don't have to; see #110 ;-)

[steffahn]

Other than the suggestions above, this PR looks good to me.

[Manishearth]

Merging this first, I'll get around to your changes soon. The reason I didn't want to make HeaderSliceWithLengthProtected contain a slice directly was that I was worried about the [T; 0] layout changes: AIUI Rust CoerceUnsized guarantees equivalent layout for Foo<[T]> vs Foo<[T; N]> but it does not do that for different types. If your changes can make that part still safe I'm all for it, I don't want the protected type to be overly generic.