[MIR] Get rid of that nasty unit_ty temporary lval #30635
Conversation
nagisa
force-pushed
the
mir-rid-unit-temp
branch
2 times, most recently
from
54cbd58 to
d3d1316
Compare
| let unit_temp = this.unit_temp.clone(); | ||
| let body_block_end = unpack!(this.into(&unit_temp, body_block, body)); | ||
| // FIXME(#30636): this should not create or request any sort of destination at | ||
| // all. |
There was a problem hiding this comment.
I'm not sure I agree with this FIXME. In trans-land, we have some special cases for "ignoring" the result and it's kind of a pain. I guess though that we could have a special mode here just for blocks, maybe.
There was a problem hiding this comment.
Also, I'm mildly worried that writing to destination here will yield incorrect results for the initialization analysis, once we implement that. Seems like something like:
let x;
x = while foo { ... /* try to use `x` in here */ }
might succeed when it shouldn't. But hmm that particular example I guess would not, since there is at least one iteration (the first one) where the body has not executed, and thus has not written into destination. Hmm, I guess it's ok.
nagisa
force-pushed
the
mir-rid-unit-temp
branch
from
d3d1316 to
5d8b3a3
Compare
| unpack!(block = this.stmts(block, stmts)); | ||
| this.into(destination, block, expr) | ||
| match expr { | ||
| Some(expr) => this.into(destination, block, expr), |
There was a problem hiding this comment.
Do we still need the impl of EvalInto for Option<ExprRef<'tcx>>? I suspect not. In any case I think perhaps we ought to remove it, since it's ambiguous what should happen on None.
|
r+ modulo the 2 nits |
nagisa
force-pushed
the
mir-rid-unit-temp
branch
2 times, most recently
from
8d19c56 to
8d3c613
Compare
|
Addressing the comments resulted in more changes (see ExprKind::If and ExprKind::Return (similar bugfix to the block one)), so I’m requesting a re-review. |
Assign a default unit value to the destinations of block expressions without trailing expression, return expressions without return value (i.e. `return;`) and conditionals without else clause.
nagisa
force-pushed
the
mir-rid-unit-temp
branch
from
8d3c613 to
3692ab6
Compare
|
@bors r+ |
|
📌 Commit 3692ab6 has been approved by |
Get rid of that nasty unit_ty temporary variable created just because it might be handy to have one around, when in reality it isn’t really that useful at all. r? @nikomatsakis Fixes #30637
Get rid of that nasty unit_ty temporary variable created just because it might be handy to have one around, when in reality it isn’t really that useful at all.
r? @nikomatsakis
Fixes #30637