[MIR] Loop bodies should not have destination of any sort

[nagisa]

Loop bodies are not supposed to have destination (loops, on the other hand, may), but since MIR code generates all blocks the same way, it results in an artefact of a never-used dummy assignment in MIR for some loops like:

loop { () }

or

while 1 == 2 { () }

Probably the best thing to do here would be to have a different ExprKind for loop bodies and ignore “returns” from blocks there.

[nikomatsakis]

The code in build/expr/category.rs already has this comment, which seems like it is pointing out roughly the same suggestion:

           ExprKind::Loop { .. } |
            ExprKind::Block { .. } |
            ExprKind::Break { .. } |
            ExprKind::Continue { .. } |
            ExprKind::Return { .. } =>
                // FIXME(#27840) these probably want their own
                // category, like "nonterminating"
                Some(Category::Rvalue(RvalueFunc::Into)),

[nagisa]

After some discussion with @nikomatsakis it seems to us that introducing a MIR pass somewhere later in the pipeline (e.g. after MIR-based-borrowck) might be a better, and more general, approach. The pass in question would remove unnecessary assignments: most notably, for this issue, the assignments of values of void (zero-sized, without destructor) type.

This and onwards is the relevant discussion.

[arielb1]

Our MIR trans code should just not lower writes to zero-sized types to anything.

[nagisa]

@arielb1 these (i.e. at least moves, in form of var1 = var0) are necessary in precense of ZST which have associated destructors.

[nikomatsakis]

@arielb1 I certainly agree about the trans code -- the question is whether MIR construction should include them. I think the answer there has to be yes. I think the trans code currently screens them out, but the point of @nagisa's comment was that maybe it'd be better to do this as a pre-pass before trans (but after our static analyses have concluded).

[nagisa]

IMO trans should also represent MIR it gets as faithfully as possible, so the graphviz you look at and LLVM IR you have at hand are as close as possible. There’s no necessity to introduce workarounds into MIR trans now what later can be implemented as an optimisation on MIR itself.

[arielb1]

@nagisa

The obvious LLVM equivalent of a zero-sized write is no code at all.

Removing pointless unit writes from the MIR might make it smaller and easier to understand, but writes of other zero-sized types may have important semantic content - that is the typical tradeoff of looking at code with/without optimizations.

Of course, if we want heavy optimizations, we would need to have both type-safe and type-unsafe versions of MIR. We also need drop-flag-containing and drop-flag-lacking versions of MIR, but I am not sure we don't want drop-flag-containing type-safe MIR.

[nagisa]

The obvious LLVM equivalent of a zero-sized write is no code at all.

Sure. However, MIR trans is not really structured in a way to let us avoid generating these, so we, currently, generate something resembling store undef, %destination, where %destination sometimes is also an undef.

Of course, if we want heavy optimizations, we would need to have both type-safe and type-unsafe versions of MIR. We also need drop-flag-containing and drop-flag-lacking versions of MIR, but I am not sure we don't want drop-flag-containing type-safe MIR.

I do not see the implication here. Why would untyped MIR be necessary for heavier optimisations?

writes of other zero-sized types may have important semantic content

I agree.

[arielb1]

Sure. However, MIR trans is not really structured in a way to let us avoid generating these, so we, currently, generate something resembling store undef, %destination, where %destination sometimes is also an undef.

Shouldn't be so hard to fix. All MIR stores are supposed to go through either store_ty, memcpy_ty, or store_fat_ptr, and memcpy_ty already skips zero-sized types.

[arielb1]

I do not see the implication here. Why would untyped MIR be necessary for heavier optimisations?

Because optimizations have a tendency of breaking type-safety, or at least borrow-safety.

[solson]

Another consequence of this is making MIR not type-correct by inserting statements like tmp1 = () where tmp1 does not have type (), see #30876.

[nikomatsakis]

Heh, actually I think that #30876 is kind of a separate thing. That basically just suggests that the hack that was added in #30635 was kind of bogus. I forgot of course that loop { } does not have type ().