Branded raw pointer const/mut correctness Was: ContiguousMut is unsafe to use, because its methods take &self.

[eddyb]

(found by @CAD97)

Both begin_mut and end_mut have this issue:

indexing/src/container_traits.rs

Lines 25 to 30 in 5549752

	fn begin_mut(&self) -> *mut Self::Item {
	self.begin() as _
	}
	fn end_mut(&self) -> *mut Self::Item {
	self.end() as _
	}

Example of usage where &mut is obtained from the resulting *mut:

indexing/src/container.rs

Lines 469 to 475 in feeb396

	fn index_mut(&mut self, r: Range<'id, P>) -> &mut Self::Output {
	unsafe {
	std::slice::from_raw_parts_mut(
	self.arr.begin_mut().offset(r.start as isize),
	r.len())
	}
	}

cc @RalfJung

[RalfJung]

Yeah that sounds wrong, you shouldn't turn an & into an &mut.

[CAD97]

Having worked with indexing a bit more, I may have jumped the gun here.

I think Container is always a wrapper around a reference. Thus iff this is *(&&mut _) as *mut _, it should be ok. It's not exactly the easiest thing to check, though.

Having looked at it again specifically for this issue: ContainerMut for &mut T delegates to ContainerMut for T, and ContainerMut for [T] exists as the terminating impl. This means that the &<[T] as ContainerMut> is turned into the mutable pointer, then mutable reference. And of course ContainerMut's default impls just shell out to the immutable Container, so...

[eddyb]

@CAD97 Container can't be a wrapper around a reference, always, because it can own e.g. Vecs.
And &&mut T can't be used to get a &mut T, it's effectively &&T.

[bluss]

I took a closer look, and I think the whole raw pointer stuff in the crate needs a remake. (The branded raw pointers are the side show of the experiment, the branded indices and index ranges the main act.)

The reason is that the PIndex<'id, T> is represented by a *const T, which should be just fine, but it is also derived from a &[T] in the slice case, and can later be converted to a *mut T and dereferenced to &mut T, which doesn't seem to be correct. Only if the container is actually mutable, of course, but still.

I think this means that PIndex needs to be split into a const and mut type, we can't use one branded raw pointer for both.

[RalfJung]

The reason is that the PIndex<'id, T> is represented by a *const T, which should be just fine, but it is also derived from a &[T] in the slice case, and can later be converted to a *mut T and dereferenced to &mut T, which doesn't seem to be correct.

Indeed, that's "mutating through a shared reference" and very forbidden. (For now, even just producing a mutable reference counts as mutation.)

Note that you can use *const T for everything, that's fine, but if you want to later mutate through a pointer you have to make sure it comes from a &mut [T] or so (and, because of rust-lang/rust#56604, right now you have to go from &mut to *mut and only then to *const -- this may or may not change eventually). In other words, it's the cast that is relevant, not the value you use for "data at rest".