Cleanup: Place::is_indirect vs Place::has_deref

[RalfJung]

Since #97077 we have these two methods is_indirect and has_deref that are documented as doing the exact same thing, but don't even mention each other in the documentation. Calling the wrong one can either lead to slow code or to subtle bugs. That's pretty bad, we should find a way to clean this up -- and at the very least, this needs better documentation.

[saethlin]

I think this is documented in Place::has_deref:

rust/compiler/rustc_middle/src/mir/mod.rs

Lines 1595 to 1603 in 474709a

	/// If MirPhase >= Derefered and if projection contains Deref,
	/// It's guaranteed to be in the first place
	pub fn has_deref(&self) -> bool {
	// To make sure this is not accidentally used in wrong mir phase
	debug_assert!(
	self.projection.is_empty() || !self.projection[1..].contains(&PlaceElem::Deref)
	);
	self.projection.first() == Some(&PlaceElem::Deref)
	}

Though why we have that assertion in only one place and not all the others, I have no idea.

[saethlin]

I tried pasting this check into is_indirect and it fails in CopyProp so yeah I have no idea 🤷

[RalfJung]

That doc comment doesn't really tell a user much, does it? It should start by describing what the function does, which is the same as is_indirect. And then it should follow "WARNING: only call this in MIR passes with MirPhase >= Derefered! It will produce wrong results afterwards." But also is_indirect should probably say "If this is called in a MIR pass with MirPhase >= Derefered, use has_deref instead, it will be faster".

One of the two should probably be renamed, there's no good reason for them to have so different names when they implement almost the same spec.

The current comment is an implementation comment that could go inside the function, but not suitable as a doc comment.

[RalfJung]

Also the methods on Place should probably just delegate to the ones on PlaceRef to avoid the duplication.

[RalfJung]

I tried pasting this check into is_indirect and it fails in CopyProp so yeah I have no idea shrug

Uh, but doesn't that run sufficiently late that it should be allowed to use has_deref?

[saethlin]

Uh, but doesn't that run sufficiently late that it should be allowed to use has_deref?

Yes. At a guess, there is MIR which is temporarily invalid during a pass.

[RalfJung]

Wow. That makes has_deref quite the footgun.

[ouz-a]

Maybe we can close this since #114505 is merged now.