Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[compiler] Factor out function effects from reference effects #30920

Merged
merged 1 commit into from
Sep 13, 2024

Commits on Sep 8, 2024

  1. [compiler] Factor out function effects from reference effects

    Summary:
    This PR performs a major refactor of InferReferenceEffects to separate out the work on marking places with Effects from inferring FunctionEffects. The behavior should be identical after this change (see [internal sync](https://www.internalfb.com/intern/everpaste/?handle=GN74VxscnUaztTYDAL8q0CRWBIxibsIXAAAB)) but the FunctionEffect logic should be easier to work with.
    
    These analyses are unfortunately still deeply linked--the FunctionEffect analysis needs to reason about the "current" value kind for each point in the program, while the InferReferenceEffects algorithm performs global updates on the state of the program (e.g. freezing). In the future, it might be possible to make these entirely separate passes if we store the ValueKind directly on places.
    
    For the most part, the logic of reference effects and function effects can be cleanly separated: for each instruction and terminal, we visit its places and infer their effects, and then we visit its places and infer any function effects that they cause. The biggest wrinkle here is that when a transitive function freeze operation occurs, it has to happen *after* inferring the function effects on the place, because otherwise we may convert a value from Context to Frozen, which will cause the ContextualMutation function effect to be converted to a ReactMutation effect too early. This can be observed in a case like this:
    
    ```
    export default component C() {
      foo(() => {
        const p = {};
        return () => {
          p['a'] = 1
        };
      });
    }
    ```
    Here when the outer function returns the inner function, it freezes the inner function which transitively freezes `p`. But before that freeze happens, we need to replay the ContextualMutation on the inner function to determine that the value is mutable in the outer context. If we froze `p` first, we would instead convert the ContextualMutation to a ReactMutation and error.
    
    To handle this, InferReferenceEffects now delays the exection of the freezeValue action until after it's called the helper functions that generate function effects. So the order of operations on a given place is now
    
    set effect --> generate function effects --> transitively freeze dependencies, if applicable
    
    [ghstack-poisoned]
    mvitousek committed Sep 8, 2024
    Configuration menu
    Copy the full SHA
    00121a4 View commit details
    Browse the repository at this point in the history