[compiler] Factor out function effects from reference effects

[mvitousek]

Stack from ghstack (oldest at bottom):

• -> [compiler] Factor out function effects from reference effects #30920

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) 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

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 8, 2024 11:29pm

[mvitousek]

As a side refactoring in this PR, I found the structure of the main IRE switch to be kind of confusing -- each case ending with continue if it's already performed all the actions it needs to, or with break if the case sets up the effect and valueKind variables to be handled in the fallthrough. This changes it to be explicitly controlled by a continuation variable (kind of a misnomer since it's not an actual function continuation, but *shrug*). All cases now break.

[josephsavona]

This looks great. Per discussion i think we should just save the ValueKind on Place in InferReferenceEffects so that we can more cleanly separate these passes, that idea also came up in the context of @mofeiZ's work on PropagateScopeDepsHIR