Effect.Capture is very similar to Effect.Read, but the only difference is that
this reference is stored somewhere via a Effect.Store.
Previously, any operand associated with a Effect.Store in the same instruction
would get aliased -- so there was no need to explicitly differentiate between a
"normal read" and "read that gets stored".
This difference is now explicit with FunctionExpression where every dependency
is "read" but only a few are "captured" for store (and mutation). In a follow up
PR, the mutating deps will have an Effect.Capture to differentiate from the
other non mutating deps (Effect.Read).
This commit adds a new Program visitor to our Babel plugin which then calls our
FunctionDeclaration visitor. Babel does some "smart" merging of plugin passes so
so even if plugin A is inserted prior to plugin B, if A does not have a Program
visitor and B does, B will run first.
Note that we also can't use Forget inside of a Babel preset as plugins run
_before_ presets (https://babeljs.io/docs/en/plugins/#plugin-ordering).
When this flag is enabled, Forget will only compile function declarations opted
in via the `'use forget'` directive. By default this is false.
Tested internally, see related diffs
Improves DCE, using fixpoint iteration to detect values that are updated across
loops but otherwise never read. There is still some further optimization we can
do (the dce-loop case could optimize out `y`), but this seems like plenty for
now.
Actually, we probably have to add some return statements to our fixtures before
landing this, because otherwise most of the code goes away.
This is a first pass at DCE without having read any literate on the subject, so
lemme know if there's a better approach. That said the algorithm is:
* Keep a `Set<Identifier>` of identifiers that are used (and whose constructing
logic cannot be removed).
* Do a first RPO iteration of all block's phis. Any phi operand that
participates in a loop is preemptively marked as "used" even if it isn't
strictly used somewhere. This step is necessary bc these operands may otherwise
not be used.
* Do a second post-order iteration of all blocks, including iterating first
their terminals, then reverse iteration of instructions, then their phis. Mark
the operands of each as used as we encounter them, and prune instructions whose
lvalue is never used.
For now I was conservative about which types of instructions can be pruned. For
example, call instructions are never pruned, even if the result of the call is
never used.
However one catch is that we currently prune instructions that cause values to
become frozen. We had planned to add runtime calls (in dev) to freeze values for
runtime enforcement, and if we want to do that we can always add these
instructions back (or replace them with explicit freeze calls).
There are a few potential next steps but we should discuss whether they're worth
it:
* Use fixpoint iteration to find exactly which operands are actually used. This
would allow us to to prune cases such as `let x = 0; while (...) { x += 1 }` eg
where there's a phi but the result is never used. Such cases should be rare in
practice though.
* Eliminate more types of instructions, eg eliminate function calls that don't
have any mutable arguments.
There's a bug in the HIR->ReactiveFunction conversion for certain categories of
compound value blocks where we replace operands (which must be a Place) with a
ReactiveValue. This approach worked in practice for lots of cases so I thought a
type coercion was safe, but then I found a case where this assumption breaks
(see new test).
The updated logic fixes the bug and is simpler. When a value block gets split up
(because there was a nested value block), instead of replacing the earlier value
in the later instructions, we append the instructions together. This can result
in some extra nesting (which if we wanted we could flatten away) but ensures
that we maintain type-safety.
A SequenceExpression currently doesn't store the InstructionId that produced its
final `.value`. This PR adds that instruction id, which is then used in the next
PR as we compose SequenceExpressions.
Just small things I noticed when looking at InferTypes. I was thinking about how
we'd adjust this pass to account for hooks, i'll probably pause that for now but
putting this up in case you like the changes. If not no big deal!
We currently lower switch case test values within the wrong scope: the test
value really should be a value block rather than a `Place`. Until then, this PR
adds a bailout for complex test values: we allow primitives and identifiers
which should cover most real-world use-cases.
No need for InferAliasForStores to know about the semantics of each instruction
anymore. It's just a simple pass that iterates over every operand and lvalue.
The FunctionExpression is special cased because it's slightly different but I
have a follow up that removes this special casing.
This doesn't change codegen as the lvalue is unused but it lets us make this
pass be semantically the same across all instructions -- "alias lvalue and
operands of an instr".
Adds limited support for UpdateExpressions (`x++`). We now support the postfix
form (`x++` ok, `++x` is a todo) and only when the argument is an identifier. We
can relax these restrictions with more work, but this PR should be sufficient
for the examples we've seen so far.
Support TypeCastExpressions — `(x: TypeAnnotation)`. This is pretty
straightforward, it's semantically identical to a raw identifier.
One catch is that our prettier config is hard-coded to use the babel-ts parser,
i wasn't sure how to make that dynamic based on the file extension so for now i
just ignored .flow.js files in our pretter config.
```
function useBar(props) {
let z;
if (props.a) {
if (props.b) {
z = baz();
}
}
return z;
}
```
Currently fails with
```
InvariantViolation: A phi cannot have two operands initialized before its
declaration
```
Changes ReactiveWhileTerminal’s test to use the new value block representation.
This means logical and condition expressions will work as while test values now.
Updates some passes from ReactiveScopes/ to use the visitor added in the
previous PR. The +124/-354 line count on this diff tells the story — the new
visitor avoids a lot of boilerplate and helps focus on the logic not the
traversal.
Note that there are a few passes which transform the function such as
adding/removing scopes. A follow-up will extend the visitor to support that and
convert the remaining passes.
This adds a truly general-purpose visitor pattern for ReactiveFunction, modeled
on what's worked well in Relay Compiler. All types of node that have children
get a visitFoo/traverseFoo pair of functions. By default the visitFoo() function
delegates to the traverseFoo() function, but the visit variant is meant to be
overridden and can delegate to the traverseFoo() variant — this gives you
precise control so that you can save/restore state before/after traversing
children.
Probably the only interesting thing is that visitLValue() does not call
visitPlace() by default though it technically could. So far that is making sense
in the passes i converted.
Note that once all passes are updated to use this, i'll delete the other visitor
helpers for ReactiveFunction.
There's a bug with assignment expression in normal value blocks due to LeaveSSA.
Until that's resolved i'm temporarily distinguishing "loop" blocks and "value"
blocks, and disallowing assignment expressions in value blocks specifically.
Sathya Gunasekaran
Lauren Tan
Joe Savona
Mofei Zhang
mofeiZ
