Commit Graph

18 Commits

Author SHA1 Message Date
Joe Savona 241b35463c Fix PruneMaybeThrows to update phi operand predecessor ids
When PruneMaybeThrows removes maybe-throw terminals, it's possible that the 
block in question reassigned a value s.t. it appears as a later phi operand. 
That phi has to be rewritten to reflect the updated predecessor block. 

Here we track these rewrites (transitively) and rewrite phi operands 
accordingly.
2024-03-21 14:11:04 -07:00
Mofei Zhang 81695f62c2 [refactor] Refactor Memoize to two instructions: Start and Finish
--- 

Previously, we always emitted `Memoize dep` instructions after the function 
expression literal and depslist instructions 

```js 

// source 

useManualMemo(() => {...}, [arg]) 

// lowered 

$0 = FunctionExpression(...) 

$1 = LoadLocal (arg) 

$2 = ArrayExpression [$1] 

$3 = Memoize (arg) 

$4 = Call / LoadLocal 

$5 = Memoize $4 

``` 

Now, we insert `Memoize dep` before the corresponding function expression 
literal: 

```js 

// lowered 

$0 = StartMemoize (arg)      <---- this moved up! 

$1 = FunctionExpression(...) 

$2 = LoadLocal (arg) 

$3 = ArrayExpression [$2] 

$4 = Call / LoadLocal 

$5 = FinishMemoize $4 

``` 

Design considerations: 

- #2663 needs to understand which lowered instructions belong to a manual 
memoization block, so we need to emit `StartMemoize` instructions before the 
`useMemo/useCallback` function argument, which contains relevant memoized 
instructions 

- we choose to insert StartMemoize instructions to (1) avoid unsafe instruction 
reordering of source and (2) to ensure that Forget output does not change when 
enabling validation 

This PR only renames `Memoize` -> `Start/FinishMemoize` and hoists 
`StartMemoize` as described. The latter may help with stricter validation for 
`useCallback`s, although testing is left to the next PR. 

#2663 contains all validation changes
2024-03-18 12:09:37 -04:00
Joe Savona 31e128a441 Use opaque type for Identifier.name for improved correctness
Uses an enum for Identifier.name to distinguish originally named identifiers vs 
promoted temporaries. An opaque type for the named identifier variant makes it 
hard to accidentally create that type.
2024-03-06 11:07:08 -08:00
Joe Savona 292a247cc4 Add type to DeclareLocal 2024-02-05 21:51:11 -08:00
Mofei Zhang 4aa60d32b9 [patch][dce] Patch dce to have separate mark and sweep phases
--- 

Previously, our logic was something like: 

```js 

fixed-point-loop { 

foreach instruction { 

mark referenced identifiers 

// assume that usages are always visited before declarations 

if (instruction is decl) { 

prune(instruction); 

} 

} 

foreach instruction { 

if not referenced { 

delete(instruction); 

} 

} 

``` 

This contained a bug, as not all usages of a variable are guaranteed to be 
visited before its declaration. 

```js 

// input 

let x = 0; 

while(x < 10) { 

x += 2; 

} 

return x; 

// hir 

entry: 

x$0 = 0 

goto loop-test 

loop-test: 

x$1 = phi(x$0, x$2) 

if ... goto loop-body else goto fallthrough 

loop-body: 

x$2 = x$1 ... 

goto loop-test 

fallthrough: 

return x$1 

``` 

In this example,`x$2` is defined by `loop-body` and used by `loop-test`. 
Similarly, `x$1` is defined by `loop-test` and used by `loop-body`. 

--- 

TODO: trying to come up with more test fixtures
2024-01-18 18:29:19 -05:00
Joe Savona 723b616c67 enablePreserveMemo treats memo deps as frozen
See discussion on #2448 for full context. In the new 
`@enablePreserveExistingMemoizationGuarantees` mode, the goal is to preserve the 
existing referential equality guarantees from the original code. #2448 lays the 
groundwork by explicitly marking the _output_ of each useMemo block as memoized, 
hinting to the compiler that the value cannot subsequently change. This ensures 
the mutable range doesn't extend _later_, possibly overlapping a hook call and 
causing memoization to gett pruned. 

This PR fixes the other direction. There are cases where free variables 
referenced in the useMemo block could have been inferred as mutated, which could 
then extend the _start_ of the range earlier past a hook: 

```javascript 

const foo = createObject(); 

useBar(); 

const baz = useMemo(() => { 

const baz = createObject(); 

maybeMutate(foo, baz); 

return baz; 

}, [foo]); 

``` 

Here the compiler would infer that both `baz` and `foo` are mutable at the 
`maybeMutate()` call, grouping them in the same scope. But that scope would span 
the `useBar()` call, and be pruned, meaning that `baz` went unmemoized. 

However, useMemo blocks shouldn't be mutating free variables. Only variables 
newly created within the useMemo block should be mutable. So this PR extends the 
feature to treat all free variables referenced in a useMemo block as frozen as 
of the block itself.
2023-12-15 13:47:22 -08:00
Joe Savona 2abd439b43 Option to preserve existing memoization guarantees
Adds an option to preserve existing memoization guarantees for values produced 
with useMemo and useCallback. We still discard the calls to these hooks, but we 
preserve the information that the value is frozen at that point in the program. 
Because these values are produced solely within the useMemo/useCallback 
callback, their mutation cannot have any interspersed hook calls. This means 
that the values mutable range will never span a hook and end at the point of the 
useMemo, ensuring that they are memoized at the same point. 

The main things that can change (relative to the orignal code) are: 

* Forget will infer a precise set of dependencies, ignoring the user-provided 
values. In practice this should only occur if the original code had a lint 
violation, which Forget would bail out on. So in practice this shouldn't happen 
unless the code doesn't use the React linter. 

* Forget may start the memoization block earlier than the developer did if other 
values are mutated along with the value being produced. This can cause 
memoization to fail, but only in situations where it would have failed 
previously: 

```javascript 

const a = []; 

useFoo(); 

const b = useMemo(() => { 

const c = a; 

c.push(1); 

return c; 

}, [a]); 

``` 

In this example (sans Forget) the useMemo will invalidate on every render 
because `a` will always be a new array and its listed as a dependency of the 
useMemo. Forget would correctly determine that the memoization would have to 
work as follows: 

```javascript 

let c; 

if (...) { 

const a = [] 

useFoo(); // OOPS we made a hook call conditional 

const t0 = a; 

t0.push(1); 

c = t0; 

... 

} else { 

c = $[...] 

} 

``` 

Because this is invalid, Forget would (later in the pipeline) strip out this 
memoization block and (as with the original) leave `c` un-memoized. 

In this same example, removing the hook would cause Forget to be able to memoize 
a value that wasn't memoized before: 

```javascript 

const a = []; 

const b = useMemo(() => { 

const c = a; 

c.push(1); 

return c; 

}, [a]); 

``` 

This invalidates every render without Forget, but would memoize correctly with 
Forget (it would expand the memoization block to include the declaration of 
`a`).
2023-12-15 13:47:20 -08:00
Mofei Zhang 995f4b0528 [bugfix] Fix constant propagation to ObjectMethods 2023-12-06 13:20:50 -05:00
Sathya Gunasekaran 6405c980eb Use starred-block for multi line comments 2023-11-08 08:27:41 +00:00
Joe Savona f8cee28f1d Retain minimal variable declarations in DCE
Currently DCE can remove variable declarations that are unused, ie where all 
control-flow paths to usage of the variable are overwritten by a reassignment. 
We then have to reconstruct the original variable declaration at the appropriate 
block scope during LeaveSSA, which is complex and can actually be incorrect in 
some cases. 

This PR updates to ensure that DCE will not remove the original variable 
declaration for any variable that is used (even in the case of always being 
reassigned before use). The main changes are: 

* DCE retains variable declarations, but if a variable declaration is always 
shadowed by reassignments then DCE will rewrite StoreLocal -> DeclareLocal so 
that it can DCE the unused initial value. 

* BuildHIR now has to change its handling for reassignment destructure 
instructions with nesting. Nesting uses a temporary which would appear as a 
declaration of a new variable, which is incompatible with other reassignments. 
See comments in the file. 

* LeaveSSA is quite a bit simpler now, since we never need to reconstruct a 
declaration.
2023-10-19 22:08:15 +01:00
Joe Savona 495fec19e7 [be] Update docblock comment for ConstantPropagation 2023-10-10 13:15:46 -07:00
Joe Savona ba23b990ac [be] Remove dead code in ConstantPropagation 2023-10-10 13:15:39 -07:00
Sathya Gunasekaran f8996476ff [hir] Wire up object method lowering and codegen
Object methods are lowered to functions and added to ObjectExpression. The 
codegen is interesting because we shouldn't emit code that lowers the object 
method into a separate statement and then stores it into an object expression. 

An shorthand object method has different semantics than an object method using 
the function syntax, so we need to preserve the shorthand object method syntax 
in the generated code. 

To do this, we don't immediately generate an AST node for the ObjectMethod but 
instead store it in a side table during codegen. Only when emitting code for an 
ObjectExpression, we lookup this side table and emit the object method inline in 
the body.
2023-10-02 19:26:00 +01:00
Sathya Gunasekaran 88f65d44a7 [hir] Refactor out LoweredFunction from FunctionExpression 2023-09-15 17:16:04 +01:00
Joe Savona ade3235ca3 ForInStatement: specialize inference/types
Replaces the use of `NextIterableOf` in for-in with a new `NextPropertyOf` 
instruction. The key distinction is `for-of` invokes an arbitrary iterator, 
which means a) each iteration may mutate the collection being iterated and b) 
the returned value may be mutable. However, `for-in` invokes a language-level 
mechanism to iterate: simply iterating alone _cannot_ modify the collection, and 
the returned value is known to be a primitive.
2023-09-11 16:54:05 -07:00
Joe Savona de2b684bc5 TryStatement: handle edge case control flows
Adds handling for some cases where the handler is unreachable (or is provably 
unreachable after analysis & optimization), where the try/catch can be flattened 
away: 

* The try block is empty. Nothing can throw, so the handler is unreachable. 

* The try block will always return. It can't return anything interesting (ie the 
result of a function call or variable load) since those could throw, but a try 
block that always return a primitive means the handler is unreachable. 

* The same, except where we only determine that the try block always returns via 
constant propagation.
2023-09-08 13:54:45 -07:00
Joe Savona ff0b05848b TryStatement: optimization pass, handle early returns
Adds an optimization pass to prune unnecessary maybe-throw terminals, when the 
block can be proven not to throw. For now we're _very_ conservative about what 
instructions we consider not to throw. There isn't too much of an advantage in 
pruning further, either. 

This PR also updates BuildReactiveFunction to handle the possibility of early 
returns within try or catch blocks, making sure we don't hit the invariant of 
emitting the same block twice.
2023-09-07 16:32:53 -07:00
lauren a0dc166991 [rfc] Remove top level forget directory
Sorry about the thrash in advance! This removes the top level `forget` directory 
which adds unnecessary nesting to our repo 

Hopefully everything still works
2023-08-22 15:04:54 -04:00