[compiler] Prevent overriding a derivationEntry on effect mutation and instead update typeOfValue and fix infinite loops

[jorge-cab]

Summary:
With this we are now comparing a snapshot of the derivationCache with the new changes every time we are done recording the derivations happening in the HIR.

We have to do this after recording everything since we still do some mutations on the cache when recording mutations.

Test Plan:
Test the following in playground:

// @validateNoDerivedComputationsInEffects_exp

function Component({ value }) {
  const [checked, setChecked] = useState('');

  useEffect(() => {
    setChecked(value === '' ? [] : value.split(','));
  }, [value]);

  return (
    <div>{checked}</div>
  )
}

This no longer causes an infinite loop.

Added a test case in the next PR in the stack

---

Stack created with Sapling. Best reviewed with ReviewStack.

• [compiler] Prevent local state source variables from depending on other state #35044
• [compiler] Don't validate when effect cleanup function depends on effect localized setState state derived values #35020
• [compiler] Switch to track setStates by aliasing and id instead of identifier names #34973
• [compiler] Update ValidateNoDerivedComputationsInEffects_exp to log the error instead of throwing #34972
• [compiler] Fix false negatives and add data flow tree to compiler error for no-deriving-state-in-effects #34995
• -> [compiler] Prevent overriding a derivationEntry on effect mutation and instead update typeOfValue and fix infinite loops #34967

[mofeiZ]

If the derivation cache does not already have this entry, we still may want to record a typeOfValue. Or is this already handled by your fixpoint iteration logic?

function useHook() {
  const [s, setS] = useState();
  const cb = () => append(arr, s);  // `arr` is mutable here and should get marked as `fromState` 
  const arr = [];
}

[jorge-cab]

Ah I see, I didn't add it because I couldn't think of an example when it would record a derivation for the first time on mutation. I'll add it

[mofeiZ]

Hmm what's the reasoning for moving when we record phis? A block's phis often defines values used by the block so we may find that the derivation cache is missing values when processing instructions with this change.

In the below example, we want to call recordPhiDerivations to insert x@2 into the context. Then, when we walk over the instructions of the fallthrough block (LoadLocal x@2, Call doSomething), we can read the proper typeOfValue from context

let x = thing1; // x@0
if (cond) {
  x = {}; // x@1
}
// x@2 = phi(x@0, x@1)
doSomething(x) // <- references x@2

[jorge-cab]

Oh, my bad, I moved it when debugging the infinite loop. I'll move it back