Custom MIR: Parse ExprKind::Use correctly

[cbeuw]

ExprKind::Use isn't parsed properly in the custom MIR builder as the fall-through case attempts to parse the whole ExprId as an operand, but only the source field inside the Use variant is.

One instance where ExprKind::Use is emitted is an identity as cast.

r? @oli-obk or @tmiasko or @JakobDegen

[JakobDegen]

Hmm, I don't think the test you added is what we want. Either we should emit an actual Rvalue::Cast or fail to parse it entirely. Deleting the cast silently seems not great. Do you have a use case for this?

[cbeuw]

we should emit an actual Rvalue::Cast

I agree, although the Cast was already removed in THIR, not here. I can look into this

fail to parse it entirely

We could prohibit identity cast in MIR entirely, though I don't see a good reason for this restriction (cc @RalfJung). It's failing currently but for the wrong reason (trying to parse a non-operand as an operand).

Do you have a use case for this?

I'm building a fuzzer that generates MIR through the custom_mir feature, and found out that it can't parse an identity cast

[JakobDegen]

We can allow it in Mir without parsing it in custom mir. That isn't great, but it's not the worst thing ever either. This of course does give your fuzzer less coverage, but I think I would rather fail to parse this entirely than parse it into what seems to me to be the wrong thing.

Edit: although, now that I say that, this PR doesn't actually fix the coverage problem so that can't really be the motivation either

[cbeuw]

Even if we accept that identity cast isn't parsed by custom MIR, we still need to parse ExprKind::Use properly as this is valid THIR that can be potentially emitted otherwise. I just need to find a test case that does this...

[RalfJung]

We currently can't have identity casts in MIR? That can't be entirely right, I can write a generic fn cast<T, U>(x: *const T) -> *const U { x as _ } and then monomorphize it so that this is the identity. So to me it seems like we have a strange optimization early in the pipeline that removes identity casts, but why would we not allow them in MIR?

[cbeuw]

We currently can't have identity casts in MIR?

Since identity casts are removed in THIR, I believe it is never actually emitted as MIR (whether compiling normally from Rust or through custom MIR). We probably should allow it, but we aren't generating it atm.

then monomorphize it so that this is the identity

Do we ever emit monomorphized MIR?

[RalfJung]

We don't manifest it, no. But conceptually this is what the interpreter and codegen are dealing with, so from their perspective identity casts already exist.

we should emit an actual Rvalue::Cast

That seems like the most sensible choice to me, to make as always emit Rvalue::Cast.

[bors]

☔ The latest upstream changes (presumably #109442) made this pull request unmergeable. Please resolve the merge conflicts.

[apiraino]

If I read the conversation correctly this PR can switch to waiting on author for some design work. Feel free to request a review with @rustbot ready, thanks!

@rustbot author

[JohnCSimon]

@cbeuw

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive