MIR: Do not require END_BLOCK to always exist #33030
Conversation
Once upon a time, along with START_BLOCK and END_BLOCK in the castle of important blocks also lived a RESUME_BLOCK (or was it UNWIND_BLOCK? Either works, I don’t remember anymore). This trinity of important blocks were required to always exist from the birth to death of the MIR-land they belonged to. Some time later, it was discovered that RESUME_BLOCK was just a lazy goon enjoying comfortable life in the light of fame of the other two. Needless to say, once found out, the RESUME_BLOCK was quickly slain and disposed of. Now, the all-seeing eye of ours discovers that END_BLOCK is actually the more evil and better disguised twin of the slain RESUME_BLOCK. Thus END_BLOCK gets slain and quickly disposed of. Glory to the START_BLOCK, one and only lord of the important blocks’ castle! --- Basically, all this does, is removing restriction for END_BLOCK to exist past the first invocation of RemoveDeadBlocks pass. This way for functions whose CFG does not reach the `END_BLOCK` end up not containing the block. As far as the implementation goes, I’m not entirely satisfied with the `BasicBlock::end_block`, I had hoped to make `new` a `const fn` and then just have a `const END_BLOCK` private to mir::build, but it turns out that constant functions don’t yet support conditionals nor a way to assert.
|
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
| pub const fn end_block() -> BasicBlock { | ||
| BasicBlock(1) | ||
| } | ||
|
|
There was a problem hiding this comment.
Why have this at all? Why not make it somehow local to the build package? Ideally, it'd just be a field of the Builder.
There was a problem hiding this comment.
(Basically, I think we should go all the way and construct the END_BLOCK lazilly.)
|
Updated |
|
So I guess that this code will still build the return block, even for a function like |
|
@bors r+ |
|
📌 Commit d1180af has been approved by |
MIR: Do not require END_BLOCK to always exist Basically, all this does, is removing restriction for END_BLOCK to exist past the first invocation of RemoveDeadBlocks pass. This way for functions whose CFG does not reach the `END_BLOCK` end up not containing the block. As far as the implementation goes, I’m not entirely satisfied with the `BasicBlock::end_block`. I had hoped to make `new` a `const fn` and then just have a `const END_BLOCK` private to mir::build, but it turns out that constant functions don’t yet support conditionals nor a way to assert.
|
💔 Test failed - auto-win-msvc-32-opt |
|
@bors: retry On Wed, Apr 20, 2016 at 11:36 PM, bors notifications@github.com wrote:
|
Basically, all this does, is removing restriction for END_BLOCK to exist past the first invocation of RemoveDeadBlocks pass. This way for functions whose CFG does not reach the
END_BLOCKend up not containing the block.As far as the implementation goes, I’m not entirely satisfied with the
BasicBlock::end_block. I had hoped to makenewaconst fnand then just have aconst END_BLOCKprivate to mir::build, but it turns out that constant functions don’t yet support conditionals nor a way to assert.