DROP to DROP_IF

[nikomatsakis]

Proposal

This is a proposal to modify the MIR such that we can run "drop elaboration" before borrow check. This eliminates duplicate logic and simplifies borrow check, moving us closer to our goal of having borrow check run against MIR that is as close as possible to the MIR that we will use for codegen. It also establishes a single "analysis MIR" that is used for both borrow check and const-promotion/const-safety, and potentially other future analyses (e.g., user-provided analyses based on the stable MIR effort).

Background: How MIR and drop-evaluation works today

In MIR today, the semantics of DROP in MIR change over time:

• After MIR build, DROP(x) means "drop x if it is initialized at runtime".
  • This allows MIR build to insert DROP(x) for every variable that is going out of scope, without doing any sort of flow-sensitive analysis.
• When borrowck runs, DROP(x) has this semantics.
  • The borrow check computes a "maybe initialized" analysis to figure out which drops may take effect and which are no-ops.
  • This is in addition to the "definitely initialized" analysis the borrow checker has to use to detect uses of (potentially) moved values.
• Drop elaboration runs, performing various refinements:
  • DROP(x) where x is not initialized are removed.
  • DROP(x) may be expanded to statements like DROP(x.y) if only some fields are initialized.
  • For conditional drops, temporary variables are introduced and DROP(x) is converted to if tmp { DROP(x) }. This requires modifying the control-flow graph in non-trivial ways.
• We then run const-eval and potentially other analyses.
• Finally, we perform optimizations and do codegen.

The MIR itself has the following drop-related terminators:

• Drop(Place) -- drop the value at place, if it is initialized; after drop-elaboration, the place will always be initialized
• DropAndReplace(Place, Value) -- drop the value at place, if it is initialized, and stuff in Value if there is a panic (I think?)

Proposal

Alter the MIR to have the following terminators:

• DropIfInit(Place) -- emitted only by MIR build, indicates we should drop Place if it is (dynamically) initialized
• DropIf(Operand, Place) -- if operand is true, drop place (which must be initialized)
• DropAndReplaceIf(Operand, Place, Value) -- if operand is true, drop place (which must be initialized)

During MIR build, we create DropIfInit.

Drop elaboration will convert to the other two, and hence most code can expect DropIfInit to never occur:

• if the drop is not not needed, it can be removed
• if only some fields are needed, the drop can be converted to DropIf(True, Place.F1) etc
• if the drop is conditional, a flag F can be introduced and we can then change to DropIf(F, Place)
  • It's possible to combine the previous two cases, of course, resulting in DropIf(F, Place.F1).
  • One side-effect of this is that drop-elaboration can be simplified: it never needs to make drastic edits to the the control-flow-graph structure, but can simply alter basic-blocks in place (though it may have to insert new blocks onto an edge, if we are going to convert DropIf(True, X) to DropIf(True, X.F) and DropIf(True, X.G)).

Borrow check will run on this "fixed" IR. It can treat the operands to DropIf as trusted, meaning that it does not verify that the flag F is true iff the value is initialized, but rather assumes that is true. Borrow check should, I believe, no longer need the "maybe initialized" analysis, however, because drop elaboration will already have removed or refined the drops using those results.

Other analyses and codegen become very mildly more complicated, in that they have to take into account that DropIf now introduces a small bit of control-flow. But this control-flow is very simple, and amounts to if !Flag { goto next_block }; else Drop(). In other words, the basic block is now an "extended basic block". I've not looked at codegen recently, so I don't know how much complexity is introduced here. If this is a problem, it could be reduced by a MIR->MIR transformation that ensures that converts DropIf(F, P) to if F { DropIf(True, P) }.

Update to the above: DropAndReplace

As discussed on Zulip, we need to investigate the best way to manage DropAndReplace. However, it is likely that we can remove it altogether in drop elaboration, just as we do today, and replace it with DropIf and an assignment. This will require borrow check to treat drop as an &mut access and not a move, but this is believed to be ok. The idea is that the borrow checker trusts that...

• DropIf statements only apply to "maybe initialized" content -- there should be no Drop or DropAndReplace statements by the time the borrow check runs.
• After a value is dropped, it will not be used again; further, drop will either (a) only be used on owned places or (b) the CFG will ensure that those places are filled again before they can possibly be used.
• The condition in DROP_IF will evaluate to true when the place is initialized or false if it is not.

Mentors or Reviewers

nikomatsakis to mentor. Giacomo Pasini is interested in implementing.

cc @oli-obk @RalfJung @ecstatic-morse and I guess @rust-lang/wg-mir-opt

This was previously discussed on Zulip around here.

Process

The main points of the Major Change Process are as follows:

• File an issue describing the proposal.
• A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

History

• Initial version of the MCP proposed generating DropIf(true) during MIR build. This was changed to a distinct DropIfInit terminator at the suggestion of @bjorn3 so as to make the constructor more robust.
• Fixed a typo in definition of DropAndReplaceIf.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

[pnkfelix]

@rustbot second

[apiraino]

@rustbot label -final-comment-period +major-change-accepted