&{mut} T may (briefly) write garbage

[nia-e]

Per opsem consensus on the topic of reference validity, expand the docs on references to account for this. Though still not explicitly settled, this also includes language allowing changing a wrapping enum discriminant (e.g. in the case of an Option<bool> handing out a &mut to the inner bool).

r? RalfJung

[rustbot]

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

[RalfJung]

Thanks! However if we land this as-is it is in direct contradiction to https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html which IMO would not be a good state to be in. I think we have to update the reference first.

When I suggested FCP'ing the discriminant meant I meant an issue in https://github.com/rust-lang/unsafe-code-guidelines/ where we can gather consensus on decisions without having to also figure out how to stably document them. :)

Cc @rust-lang/opsem

[RalfJung]

rust-lang/unsafe-code-guidelines#621 has passed FCP. I guess that means we can proceed here, though I'll nominate to make sure everyone is on board.

[RalfJung]

This is a monster sentence.

Suggested change

	/// Unsafe code is allowed to temporarily violate type validity invariants on internal data that
	/// cannot be otherwise observed; that is, arbitrary data can be written into the pointed-to bytes
	/// of a `&mut T` even if it would constitute an invalid value of `T` or set the discriminant of a
	/// wrapping enum to an invalid value, so long as everything is restored before the code returns.
	/// Unsafe code is allowed to *temporarily* violate type validity invariants on internal data that
	/// cannot be otherwise observed; that is, arbitrary data can be written into the pointed-to bytes
	/// of a `&mut T` even if it would constitute an invalid value of `T`, so long as everything is restored
	/// before the data might be used at type `T` again (in particular, before control is passed to "outside"
	/// code that has access to the data this reference points to).
	/// This is allowed even if the temporary value sets the discriminant of a
	/// wrapping enum to an invalid value.

Also can you make "pointed-to bytes" link to this?

View changes since the review

[RalfJung]

Also pinging @rust-lang/lang -- this is stably documenting the opsem consensus from rust-lang/unsafe-code-guidelines#621.

[traviscross]

Thanks! However if we land this as-is it is in direct contradiction to https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html which IMO would not be a good state to be in. I think we have to update the reference first.

...

rust-lang/unsafe-code-guidelines#621 has passed FCP. I guess that means we can proceed here, though I'll nominate to make sure everyone is on board.

Presumably we want to merge rust-lang/reference#2338 first (or break out the relevant part of it and merge that). And do we need to say more than rust-lang/reference#2338 does to fully cover what's here? (I'd prefer we fully capture these rules in the Reference.) (Also pending is rust-lang/reference#2337.) You had said you weren't ready for nominations yet on rust-lang/reference#2338. Please let me know how you would prefer to proceed on the Reference side.

(It will probably make sense to do the lang nominations on the relevant Reference PRs.)

[RalfJung]

(I'd prefer we fully capture these rules in the Reference.)

Yeah that is the part I am wondering about. I am not sure how to best do that.

The Reference barely covers our opsem. It only lists things that we consider forbidden, on the UB page. This PR starts at the other end and says that something is allowed. That doesn't really fit what we currently have in the Reference.

I was hoping we'd get rust-lang/reference#2338 in, which would certainly be enough to back this PR. But some people really want maximally strict subobject provenance, and while there is a range of subobject provenance I can live with, the maximally strict extreme end of the spectrum is not in that range. But if we're not talking about subobject provenance at all in the Reference then I don't know where we can talk about this specific concern related to enum discriminants.

[RalfJung]

If the Reference had a section on "code we consider to be sound" (as also mentioned in #161168), we could add something like this to that section:

pub fn temporary_invalid(x: &mut NonZeroI32) {
  let ptr = x as *mut _ as *mut i32;
  x.write(0);
  x.write(1);
}

Maybe that's a way to do this -- though this would only be a special case obviously.

[traviscross]

If the Reference had a section on "code we consider to be sound" (as also mentioned in #161168), we could add something like this to that section... Maybe that's a way to do this -- though this would only be a special case obviously.

Makes sense. We have a Behavior Considered Undefined chapter; we could add a sibling Behavior Considered Defined chapter.

Of course, it'd also be worth considering whether this might make sense on the Pointer Types page. We document other opsem details there.