fix behaviour of closed-over locals in and/or expressions
#1278
Conversation
`var`/`let` bindings within `and`/`or` expression are embedded into the surrounding scope, which would cease to be the case after lowering, for example, `x and y` into `(var tmp = x; (;if tmp: tmp = y); tmp), as `y`'s lifetime is then delimited by the `if`'s scope. So far, this problems was partially addressed by the "unscoped def" handling in `cgirgen`, but only for locals not lifted into the environment. Performing the hoisting in `transf` makes sure lifted locals work as expected, and it also makes sure the code works correctly without `cgirgen`.
| # lifetime earlier. That is, for ``var x = (var y = 0; y)`` the | ||
| # ``var y`` must be hoisted first | ||
| let src = hoist(c, move it[2], target) | ||
| target.add newTreeI(n.kind, it.info, |
There was a problem hiding this comment.
When n.kind is an nkLetSection aren't we going to cause potential issues downstream when assigning via such a handle?
Not really expecting a change, seeing as it does work, given the test; at most a comment, but not sure it requires that.
There was a problem hiding this comment.
That's a good question, thank you for asking. Including and beyond transf, let and var sections are effectively interchangeable (only the symbol kind matters). A let binding not being assigned a value immediately is also fine.
What would be a problem is if a let location is either read from before it's assigned to, or assigned to multiple times (mirgen relies on both to never happen), but the hoisting cannot cause either.
There was a problem hiding this comment.
Thanks for clarifying, that was my guess. I also think it's good in terms of long term direction, because making let, or some variant, to be an initialize once before use, rather than once at the time of definition, would be nice.
|
/merge |
|
Merge requested by: @saem Contents after the first section break of the PR description has been removed and preserved below:
|
Summary
Fix closed-over locals defined as part of
and/orexpressions nothaving their default value when their defined-in sub-expression was
short-circuited.
Details
Bindings part of
and/orexpressions were so far hoisted to thestart of their scope by the "unscoped def" handling in
cgirgen, whichdoesn't apply to lifted locals, since these don't have a
def(they'repart of the environment object).
Performing the hoisting in
transfmakes sure the behaviour of closed-over locals matches that of not closed-over ones, and it also removes
the reliance on
cgirgen.The hoisting works by scanning transformed
and/orexpressions forbindings that are within the same scope as the
and/orexpression,and moving them to the start of the
and/orexpression.For example,
(let a = 0; x) or (let b = 0; y)is transformed into(let a; let b; (a = 0; x) or (b = 0; y)).Lifted globals (
.globaland.thread) have to be accounted for bythe hoisting pass, since they weren't lifted at this point.