See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
See comments in code. The idea is that rather than immediately processing function expression effects when declaring the function, we record Capture effects for context variables that may be captured/mutated in the function. Then, transitive mutations of the function value itself will extend the range of these values via the normal captured value comutation inference established earlier in the stack (if capture a -> b, then transitiveMutate(b) => mutate(a)). So capture contextVar -> function and transitiveMutate(function) => mutate(contextVar).
[ghstack-poisoned]
ImmutableCapture allows us to record information flow for escape analysis purposes, without impacting mutable range analysis. All of Alias, Capture, and CreateFrom now downgrade to ImmutableCapture if the `from` value is frozen. Globals and primitives are conceptually copy types and don't record any information flow at all. I guess we could add a `Copy` effect if we wanted but i haven't seen a need for it yet.
Related, previously CreateFrom was always paired with Capture when constructing effects. But I had also coded up the inference to sort of treat them the same. So this diff makes that official, and means CreateFrom is a valid third way to represent data flow and not just creation. When applied, CreateFrom will turn into: ImmutableCapture for frozen values, Capture for mutable values, and disappear for globals/primitives.
A final tweak is to change range inference to ensure that range.start is non-zero if the value is mutated.
[ghstack-poisoned]
ImmutableCapture allows us to record information flow for escape analysis purposes, without impacting mutable range analysis. All of Alias, Capture, and CreateFrom now downgrade to ImmutableCapture if the `from` value is frozen. Globals and primitives are conceptually copy types and don't record any information flow at all. I guess we could add a `Copy` effect if we wanted but i haven't seen a need for it yet.
Related, previously CreateFrom was always paired with Capture when constructing effects. But I had also coded up the inference to sort of treat them the same. So this diff makes that official, and means CreateFrom is a valid third way to represent data flow and not just creation. When applied, CreateFrom will turn into: ImmutableCapture for frozen values, Capture for mutable values, and disappear for globals/primitives.
A final tweak is to change range inference to ensure that range.start is non-zero if the value is mutated.
[ghstack-poisoned]
ImmutableCapture allows us to record information flow for escape analysis purposes, without impacting mutable range analysis. All of Alias, Capture, and CreateFrom now downgrade to ImmutableCapture if the `from` value is frozen. Globals and primitives are conceptually copy types and don't record any information flow at all. I guess we could add a `Copy` effect if we wanted but i haven't seen a need for it yet.
Related, previously CreateFrom was always paired with Capture when constructing effects. But I had also coded up the inference to sort of treat them the same. So this diff makes that official, and means CreateFrom is a valid third way to represent data flow and not just creation. When applied, CreateFrom will turn into: ImmutableCapture for frozen values, Capture for mutable values, and disappear for globals/primitives.
A final tweak is to change range inference to ensure that range.start is non-zero if the value is mutated.
[ghstack-poisoned]
Distinguish the various forms of mutation, and do basic replaying of these effects when the function is created (imperfect, temporary).
[ghstack-poisoned]
Distinguish the various forms of mutation, and do basic replaying of these effects when the function is created (imperfect, temporary).
[ghstack-poisoned]
Distinguish the various forms of mutation, and do basic replaying of these effects when the function is created (imperfect, temporary).
[ghstack-poisoned]
This PR gets a first fixture working end-to-end with the new mutability and aliasing model. Key changes:
* Add a feature flag to enable the model. When enabled we no longer call InferReferenceEffects or InferMutableRanges, and instead use the new equivalents.
* Adds a pass that infers Place-specific effects based on mutable ranges and instruction effects. This is necessary to satisfy existing code that requires operand effects to be populated.
* Adds a pass that infers the outwardly-visible capturing/aliasing behavior of a function expression. The idea is that this can bubble up and be used in conjunction with the `Apply` effect to get precise inference of things like `array.map(() => { ... })`.
[ghstack-poisoned]
This PR gets a first fixture working end-to-end with the new mutability and aliasing model. Key changes:
* Add a feature flag to enable the model. When enabled we no longer call InferReferenceEffects or InferMutableRanges, and instead use the new equivalents.
* Adds a pass that infers Place-specific effects based on mutable ranges and instruction effects. This is necessary to satisfy existing code that requires operand effects to be populated.
* Adds a pass that infers the outwardly-visible capturing/aliasing behavior of a function expression. The idea is that this can bubble up and be used in conjunction with the `Apply` effect to get precise inference of things like `array.map(() => { ... })`.
[ghstack-poisoned]
This PR gets a first fixture working end-to-end with the new mutability and aliasing model. Key changes:
* Add a feature flag to enable the model. When enabled we no longer call InferReferenceEffects or InferMutableRanges, and instead use the new equivalents.
* Adds a pass that infers Place-specific effects based on mutable ranges and instruction effects. This is necessary to satisfy existing code that requires operand effects to be populated.
* Adds a pass that infers the outwardly-visible capturing/aliasing behavior of a function expression. The idea is that this can bubble up and be used in conjunction with the `Apply` effect to get precise inference of things like `array.map(() => { ... })`.
[ghstack-poisoned]
Alternate take at a new mutability and alising model, aiming to replace `InferReferenceEffects` and `InferMutableRanges`. My initial passes at this were more complicated than necessary, and I've iterated to refine and distill this down to the core concepts. There are two effects that track information flow: `capture` and `alias`:
* Given distinct values A and B. After capture A -> B, mutate(B) does *not* modify A. This more precisely captures the semantic of the previous `Store` effect. As an example, `array.push(item)` has the effect `capture item -> array` and `mutate(array)`. The array is modified, not the item.
* Given distinct values A and B. After alias A -> B, mutate(B) *does* modify A. This is because B now refers to the same value as A.
* Given distinct values A and B, after *either* capture A -> B *or* alias A -> B, transitiveMutate(B) counts as a mutation of A. Transitive mutation is the default, and places that previously used `Store` will use non-transitive mutate effects instead.
Conceptually "capture A -> B" means that a reference to A was "captured" (or stored) within A, but there is not a directly aliasing. Whereas "alias A -> B" means literal value aliasing.
The idea is that our previous sequential fixpoint loops in InferMutableRanges can instead work by first looking at transitive mutations, then look at non-transitive mutations. And aliasing groups can be built purely based on the `alias` effect.
Lots more to do here but the structure is coming together.
[ghstack-poisoned]
Alternate take at a new mutability and alising model, aiming to replace `InferReferenceEffects` and `InferMutableRanges`. My initial passes at this were more complicated than necessary, and I've iterated to refine and distill this down to the core concepts. There are two effects that track information flow: `capture` and `alias`:
* Given distinct values A and B. After capture A -> B, mutate(B) does *not* modify A. This more precisely captures the semantic of the previous `Store` effect. As an example, `array.push(item)` has the effect `capture item -> array` and `mutate(array)`. The array is modified, not the item.
* Given distinct values A and B. After alias A -> B, mutate(B) *does* modify A. This is because B now refers to the same value as A.
* Given distinct values A and B, after *either* capture A -> B *or* alias A -> B, transitiveMutate(B) counts as a mutation of A. Transitive mutation is the default, and places that previously used `Store` will use non-transitive mutate effects instead.
Conceptually "capture A -> B" means that a reference to A was "captured" (or stored) within A, but there is not a directly aliasing. Whereas "alias A -> B" means literal value aliasing.
The idea is that our previous sequential fixpoint loops in InferMutableRanges can instead work by first looking at transitive mutations, then look at non-transitive mutations. And aliasing groups can be built purely based on the `alias` effect.
Lots more to do here but the structure is coming together.
[ghstack-poisoned]
Alternate take at a new mutability and alising model, aiming to replace `InferReferenceEffects` and `InferMutableRanges`. My initial passes at this were more complicated than necessary, and I've iterated to refine and distill this down to the core concepts. There are two effects that track information flow: `capture` and `alias`:
* Given distinct values A and B. After capture A -> B, mutate(B) does *not* modify A. This more precisely captures the semantic of the previous `Store` effect. As an example, `array.push(item)` has the effect `capture item -> array` and `mutate(array)`. The array is modified, not the item.
* Given distinct values A and B. After alias A -> B, mutate(B) *does* modify A. This is because B now refers to the same value as A.
* Given distinct values A and B, after *either* capture A -> B *or* alias A -> B, transitiveMutate(B) counts as a mutation of A. Transitive mutation is the default, and places that previously used `Store` will use non-transitive mutate effects instead.
Conceptually "capture A -> B" means that a reference to A was "captured" (or stored) within A, but there is not a directly aliasing. Whereas "alias A -> B" means literal value aliasing.
The idea is that our previous sequential fixpoint loops in InferMutableRanges can instead work by first looking at transitive mutations, then look at non-transitive mutations. And aliasing groups can be built purely based on the `alias` effect.
Lots more to do here but the structure is coming together.
[ghstack-poisoned]
Alternate take at a new mutability and alising model, aiming to replace `InferReferenceEffects` and `InferMutableRanges`. My initial passes at this were more complicated than necessary, and I've iterated to refine and distill this down to the core concepts. There are two effects that track information flow: `capture` and `alias`:
* Given distinct values A and B. After capture A -> B, mutate(B) does *not* modify A. This more precisely captures the semantic of the previous `Store` effect. As an example, `array.push(item)` has the effect `capture item -> array` and `mutate(array)`. The array is modified, not the item.
* Given distinct values A and B. After alias A -> B, mutate(B) *does* modify A. This is because B now refers to the same value as A.
* Given distinct values A and B, after *either* capture A -> B *or* alias A -> B, transitiveMutate(B) counts as a mutation of A. Transitive mutation is the default, and places that previously used `Store` will use non-transitive mutate effects instead.
Conceptually "capture A -> B" means that a reference to A was "captured" (or stored) within A, but there is not a directly aliasing. Whereas "alias A -> B" means literal value aliasing.
The idea is that our previous sequential fixpoint loops in InferMutableRanges can instead work by first looking at transitive mutations, then look at non-transitive mutations. And aliasing groups can be built purely based on the `alias` effect.
Lots more to do here but the structure is coming together.
[ghstack-poisoned]
Alternate take at a new mutability and alising model, aiming to replace `InferReferenceEffects` and `InferMutableRanges`. My initial passes at this were more complicated than necessary, and I've iterated to refine and distill this down to the core concepts. There are two effects that track information flow: `capture` and `alias`:
* Given distinct values A and B. After capture A -> B, mutate(B) does *not* modify A. This more precisely captures the semantic of the previous `Store` effect. As an example, `array.push(item)` has the effect `capture item -> array` and `mutate(array)`. The array is modified, not the item.
* Given distinct values A and B. After alias A -> B, mutate(B) *does* modify A. This is because B now refers to the same value as A.
* Given distinct values A and B, after *either* capture A -> B *or* alias A -> B, transitiveMutate(B) counts as a mutation of A.
Conceptually "capture A -> B" means that a reference to A was "captured" (or stored) within A, but there is not a directly aliasing. Whereas "alias A -> B" means literal value aliasing.
The idea is that our previous sequential fixpoint loops in InferMutableRanges can instead work by first looking at transitive mutations, then look at non-transitive mutations. And aliasing groups can be built purely based on the `alias` effect.
Lots more to do here but the structure is coming together.
[ghstack-poisoned]
Syncing this stack internally there is a small percentage of files that lose memoization, generally for callbacks. The repro here tries to get at the core pattern, where a parameter escapes into a mutable return value. This makes the callback appear mutable, and means that calls like array.map aren't able to optimize as well — even if the array itself is transitively immutable.
The challenge is that we can't really distinguish between just capturing and true mutation right now — AnalyzeFunctions kind of has to pick one, and consider both a mutation.
[ghstack-poisoned]
Syncing this stack internally there is a small percentage of files that lose memoization, generally for callbacks. The repro here tries to get at the core pattern, where a parameter escapes into a mutable return value. This makes the callback appear mutable, and means that calls like array.map aren't able to optimize as well — even if the array itself is transitively immutable.
The challenge is that we can't really distinguish between just capturing and true mutation right now — AnalyzeFunctions kind of has to pick one, and consider both a mutation.
[ghstack-poisoned]
Syncing this stack internally there is a small percentage of files that lose memoization, generally for callbacks. The repro here tries to get at the core pattern, where a parameter escapes into a mutable return value. This makes the callback appear mutable, and means that calls like array.map aren't able to optimize as well — even if the array itself is transitively immutable.
The challenge is that we can't really distinguish between just capturing and true mutation right now — AnalyzeFunctions kind of has to pick one, and consider both a mutation.
[ghstack-poisoned]
Syncing this stack internally there is a small percentage of files that lose memoization, generally for callbacks. The repro here tries to get at the core pattern, where a parameter escapes into a mutable return value. This makes the callback appear mutable, and means that calls like array.map aren't able to optimize as well — even if the array itself is transitively immutable.
The challenge is that we can't really distinguish between just capturing and true mutation right now — AnalyzeFunctions kind of has to pick one, and consider both a mutation.
[ghstack-poisoned]
Syncing this stack internally there is a small percentage of files that lose memoization, generally for callbacks. The repro here tries to get at the core pattern, where a parameter escapes into a mutable return value. This makes the callback appear mutable, and means that calls like array.map aren't able to optimize as well — even if the array itself is transitively immutable.
The challenge is that we can't really distinguish between just capturing and true mutation right now — AnalyzeFunctions kind of has to pick one, and consider both a mutation.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Fix for the issue in the previous PR. Long-term the ideal thing would be to make InferMutableRanges smarter about Store effects, and recognize that they are also transitive mutations of whatever was captured into the object. So in the following:
```
const x = {y: {z: {}}};
x.y.z.key = value;
```
That the `PropertyStore z . 'key' = value` is a transitive mutation of x and all three object expressions (x, x.y, x.y.z).
But for now it's simpler to stick to the original idea of Store only counting if we know that the type is an object.
[ghstack-poisoned]
Adds fixture tests to demonstrate an issue in changing PropertyStore to always have a Store effect on its object operand, regardless of the operand type. The issue is that if we're doing a PropertyStore on a nested value, that has be considered a transitive mutation of the parent object:
```
const x = {y: {z: {}}};
x.y.z.key = 'value'; // this has to be a mutation of `x`
```
Fix in the next PR.
[ghstack-poisoned]
We've occassionally added logic that extends mutable ranges into InferReactiveScopeVariables to handle a specific case, but inevitably discover that the logic needs to be part of the InferMutableRanges fixpoint loop. That happened in the past with extending the range of phi operands to account for subsequent mutations, which I moved to InferMutableRanges a while back. But InferReactiveScopeVariables also has logic to group co-mutations in the same scope, which also extends ranges of the co-mutating operands to have the same end point. Recently mofeiz found some cases where this is insufficient, where a closure captures a value that could change via a co-mutation, and where failure to extend the ranges in the fixpoint meant the function expression appeared independently memoizable when it wasn't.
The fix is to make InferMutableRanges update ranges to account for co-mutations. That is relatively straightforward, but not enough! The problem is that the fixpoint loop stopped once the alias sets coalesced, but co-mutations only affect ranges and not aliases. So the other part of the fix is to have the fixpoint condition use a custom canonicalization that describes each identifiers root _and_ the mutable range of that root.
[ghstack-poisoned]
We've occassionally added logic that extends mutable ranges into InferReactiveScopeVariables to handle a specific case, but inevitably discover that the logic needs to be part of the InferMutableRanges fixpoint loop. That happened in the past with extending the range of phi operands to account for subsequent mutations, which I moved to InferMutableRanges a while back. But InferReactiveScopeVariables also has logic to group co-mutations in the same scope, which also extends ranges of the co-mutating operands to have the same end point. Recently mofeiz found some cases where this is insufficient, where a closure captures a value that could change via a co-mutation, and where failure to extend the ranges in the fixpoint meant the function expression appeared independently memoizable when it wasn't.
The fix is to make InferMutableRanges update ranges to account for co-mutations. That is relatively straightforward, but not enough! The problem is that the fixpoint loop stopped once the alias sets coalesced, but co-mutations only affect ranges and not aliases. So the other part of the fix is to have the fixpoint condition use a custom canonicalization that describes each identifiers root _and_ the mutable range of that root.
[ghstack-poisoned]
We've occassionally added logic that extends mutable ranges into InferReactiveScopeVariables to handle a specific case, but inevitably discover that the logic needs to be part of the InferMutableRanges fixpoint loop. That happened in the past with extending the range of phi operands to account for subsequent mutations, which I moved to InferMutableRanges a while back. But InferReactiveScopeVariables also has logic to group co-mutations in the same scope, which also extends ranges of the co-mutating operands to have the same end point. Recently mofeiz found some cases where this is insufficient, where a closure captures a value that could change via a co-mutation, and where failure to extend the ranges in the fixpoint meant the function expression appeared independently memoizable when it wasn't.
The fix is to make InferMutableRanges update ranges to account for co-mutations. That is relatively straightforward, but not enough! The problem is that the fixpoint loop stopped once the alias sets coalesced, but co-mutations only affect ranges and not aliases. So the other part of the fix is to have the fixpoint condition use a custom canonicalization that describes each identifiers root _and_ the mutable range of that root.
[ghstack-poisoned]
We've occassionally added logic that extends mutable ranges into InferReactiveScopeVariables to handle a specific case, but inevitably discover that the logic needs to be part of the InferMutableRanges fixpoint loop. That happened in the past with extending the range of phi operands to account for subsequent mutations, which I moved to InferMutableRanges a while back. But InferReactiveScopeVariables also has logic to group co-mutations in the same scope, which also extends ranges of the co-mutating operands to have the same end point. Recently mofeiz found some cases where this is insufficient, where a closure captures a value that could change via a co-mutation, and where failure to extend the ranges in the fixpoint meant the function expression appeared independently memoizable when it wasn't.
The fix is to make InferMutableRanges update ranges to account for co-mutations. That is relatively straightforward, but not enough! The problem is that the fixpoint loop stopped once the alias sets coalesced, but co-mutations only affect ranges and not aliases. So the other part of the fix is to have the fixpoint condition use a custom canonicalization that describes each identifiers root _and_ the mutable range of that root.
[ghstack-poisoned]
We've occassionally added logic that extends mutable ranges into InferReactiveScopeVariables to handle a specific case, but inevitably discover that the logic needs to be part of the InferMutableRanges fixpoint loop. That happened in the past with extending the range of phi operands to account for subsequent mutations, which I moved to InferMutableRanges a while back. But InferReactiveScopeVariables also has logic to group co-mutations in the same scope, which also extends ranges of the co-mutating operands to have the same end point. Recently @mofeiz found some cases where this is insufficient, where a closure captures a value that could change via a co-mutation, and where failure to extend the ranges in the fixpoint meant the function expression appeared independently memoizable when it wasn't.
The fix is to make InferMutableRanges update ranges to account for co-mutations. That is relatively straightforward, but not enough! The problem is that the fixpoint loop stopped once the alias sets coalesced, but co-mutations only affect ranges and not aliases. So the other part of the fix is to have the fixpoint condition use a custom canonicalization that describes each identifiers root _and_ the mutable range of that root.
[ghstack-poisoned]
This is a stab at addressing a pattern that mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
This is a stab at addressing a pattern that mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
This is a stab at addressing a pattern that mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
This is a stab at addressing a pattern that mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
This is a stab at addressing a pattern that mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
This is a stab at addressing a pattern that mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
This is a stab at addressing a pattern that @mofeiz and I have both stumbled across. Today, FunctionExpression's context list describes values from the outer context that are accessed in the function, and with what effect they were accessed. This allows us to describe the fact that a value from the outer context is known to be mutated inside a function expression, or is known to be captured (aliased) into some other value in the function expression. However, the basic `Effect` kind is insufficient to describe the full semantics. Notably, it doesn't let us describe more complex aliasing relationships.
From an example @mofeiz added:
```js
const x = {};
const y = {};
const f = () => {
const a = [y];
const b = x;
// this sets y.x = x
a[0].x = b;
}
f();
mutate(y.x); // which means this mutates x!
```
Here, the Effect on the context operands are `[mutate y, read x]`. The `mutate y` is bc of the array push. But the `read x` is surprising — `x` is captured into `y`, but there is no subsequent mutation of y or x, so we consider this a read. But as the comments indicate, the final line mutates x! We need to reflect the fact that even though x isn't mutated inside the function, it is aliased into y, such that if y is subsequently mutated that this should count as a mutation of x too.
The idea of this PR is to extend the FunctionEffect type with a CaptureEffect variant which lists out the aliasing groups that occur inside the function expression. This allows us to bubble up the results of alias analysis from inside a function. The idea is to:
* Return the alias sets from InferMutableRanges
* Augment them with capturing of the form above, handling cases such as the `a[0].x = b`
* For each alias group, record a CaptureEffect for any group that contains 2+ context operands
* Extend the alias sets in the _outer_ function with the CaptureEffect sets from FunctionExpression/ObjectMethod instructions.
This isn't quite right yet, just sharing early hacking.
[ghstack-poisoned]
The issue in the previous PR was due to a ContextMutation function effect having a place that wasn't one of the functions' context variables. What was happening is that the `getContextRefOperand()` helper wasn't following aliases. If an operand had a context type, we recorded the operand as the context place — but instead we should be looking through to the context places of the abstract value.
With this change the fixture now fails for a different reason — we infer this as a mutation of `params` and reject it because `params` is frozen (hook return value). This case is clearly a false positive: the mutation is on the outer, new `nextParams` object and can't possibly mutate `params`. Need to think more about what to do here but this is clearly more precise in terms of which variable we record as the context variable.
[ghstack-poisoned]
The issue in the previous PR was due to a ContextMutation function effect having a place that wasn't one of the functions' context variables. What was happening is that the `getContextRefOperand()` helper wasn't following aliases. If an operand had a context type, we recorded the operand as the context place — but instead we should be looking through to the context places of the abstract value.
With this change the fixture now fails for a different reason — we infer this as a mutation of `params` and reject it because `params` is frozen (hook return value). This case is clearly a false positive: the mutation is on the outer, new `nextParams` object and can't possibly mutate `params`. Need to think more about what to do here but this is clearly more precise in terms of which variable we record as the context variable.
[ghstack-poisoned]
The issue in the previous PR was due to a ContextMutation function effect having a place that wasn't one of the functions' context variables. What was happening is that the `getContextRefOperand()` helper wasn't following aliases. If an operand had a context type, we recorded the operand as the context place — but instead we should be looking through to the context places of the abstract value.
With this change the fixture now fails for a different reason — we infer this as a mutation of `params` and reject it because `params` is frozen (hook return value). This case is clearly a false positive: the mutation is on the outer, new `nextParams` object and can't possibly mutate `params`. Need to think more about what to do here but this is clearly more precise in terms of which variable we record as the context variable.
[ghstack-poisoned]
Found when testing the new validation from #33079 internally. I haven't fully debugged, but somehow the combination of the effect function *accessing* a ref and also calling a second function which has a purely local mutation triggers the validation. Even though the called second function only mutates local variables. If i remove the ref access in the effect function, the error goes away.
Anyway I'll keep debugging, putting up a repro for now.
[ghstack-poisoned]
Found when testing the new validation from #33079 internally. I haven't fully debugged, but somehow the combination of the effect function *accessing* a ref and also calling a second function which has a purely local mutation triggers the validation. Even though the called second function only mutates local variables. If i remove the ref access in the effect function, the error goes away.
Anyway I'll keep debugging, putting up a repro for now.
[ghstack-poisoned]
2025-05-03 09:58:15 +09:00
54 changed files with 3402 additions and 429 deletions
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
