Document and correct assignment order for yield and inline-asm - #160777
Document and correct assignment order for yield and inline-asm#160777cjgillot wants to merge 3 commits into
Conversation
|
This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @vakaras |
rustbot
added
S-waiting-on-review
|
r? @mu001999 rustbot has assigned @mu001999. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| /// The operands for the inline assembly, as `Operand`s or `Place`s. | ||
| /// | ||
| /// For the purposes of MIR analyzes, `out` and `inout` operands are only considered | ||
| /// written-to when execution continues with `targets`. |
There was a problem hiding this comment.
I don't understand this comment, which confusion is it trying to avoid?
There was a problem hiding this comment.
I want to convey that out and inout operands in inline asm work the same way. MIR analyses consider they are only written-to if the asm does not unwind.
There was a problem hiding this comment.
| /// written-to when execution continues with `targets`. | |
| /// written-to when execution continues with `targets`, but not when the asm unwinds. |
Doesn't that mean "it is UB to write to these if the asm ends up unwinding rather than continuing with one of the targets"? That seems more clear to me than talking about this in terms of what analyses do.
There are no docs changes for yields that I can find, so the docs were already correct there? |
|
r? RalfJung |
|
|
|
I had a look at the docs but I cannot review the code changes.
@rustbot reroll
|
This variant is never constructed.
For `yield` this is not a problem, as nobody uses droppable types a coroutine resume argument. For `inline_asm`, it is delicate to gauge the effect.
Yes. I changed them in #156875 |
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
5 participants
The first two commits cleanup and rename the variants of
InitKind.The third commit attempts to make several parts of MIR analyses a bit more consistent.
For calls, we correctly model assignments to happen on the return edge.
For yields, assignments also happen on the return edge according to dataflow analyses (see
TerminatorEdges), but were considered unconditional. This should not change anything, as nobody uses droppable types as coroutine resume argument.For
inline_asm, same thing, dataflow analyses consider assignments to happen ontargetsand not on unwind. This is also corrected. I have no idea what's the impact of this. I suppose user wisely avoid to assign droppable types in inline asm that may unwind.