mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
Update base for Update on "[compiler][newinference] Fix for phi types, extracting primitives from objects"
First a quick fix: if we have a known type for the lvalue of CreateFrom, we can drop the effect. This is a bit awkward generally because the types and abstract values overlap a bit, and i'd prefer to only look at types during one phase. So maybe move all the type-checking to where we generate effects, and then after that applyEffect() doesn't have to consider types. The larger fix is for InferMutationAliasingRanges. When processing a mutation we have to walk the graph in both forward and backwards directions. Consider `alias a -> b, mutate(b)`. We have to walk back the alias chain from b to a, and mark a as mutated too. But for `alias a -> b, mutate(a)`, we also have to mark b as mutated — walking forwards along the alias chain. But phis are a bit different. You can have a mutation of one of the phi operands, such that you have `a, b -> phi c, mutate(a)`. Here, we do need to mark c as mutated, but we should not walk back to b — it's a different operand! a and b don't alias together. There are now about 150 fixtures failing, but they're in a few categories and all of them are addressable: * Infinite loops. `applyEffect()` creates new un-memoized effect values which means that any input with a backedge (loop) will spin until my infinite loop detection throws. This is somewhat tedious to address but it's a pragmatic concern and not a flaw in the model. I also need to convince myself that the approach in InferMutationAliasingRanges is safe for loops, but first i have to get inputs w loops to even reach that phase. * LoadContext/StoreContext - i currently treat these too similarly to regular load/store, ie assuming the mutations happen in order. One idea is to treat LoadContext as a mutate instead of an alias, just to make sure all instances get grouped together. * InvalidReact detection. We already synthesize MutateFrozen/MutateGlobal effects but we don't throw these as errors yet. This is probably the largest category of failing tests, which means overall this is actually "pretty close" (so, 50% of the way there). [ghstack-poisoned]
This commit is contained in: