From 724b324b966343f08ff0cb16ec9d8013e3891dfd Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 15 Aug 2025 15:05:29 -0700 Subject: [PATCH 1/5] [compiler] Add hint to name variables with "Ref" suffix (#34125) If you have a ref that the compiler doesn't know is a ref (say, a value returned from a custom hook) and try to assign its `.current = ...`, we currently fail with a generic error that hook return values are not mutable. However, an assignment to `.current` specifically is a very strong hint that the value is likely to be a ref. So in this PR, we track the reason for the mutation and if it ends up being an error, we use it to show an additional hint to the user. See the fixture for an example of the message. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34125). * #34126 * __->__ #34125 * #34124 --- .../src/Inference/AliasingEffects.ts | 4 +- .../Inference/InferMutationAliasingEffects.ts | 75 ++++++++++++++----- ...-assing-to-ref-current-in-render.expect.md | 42 +++++++++++ ...invalid-assing-to-ref-current-in-render.js | 7 ++ 4 files changed, 107 insertions(+), 21 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts index c492e2835460..80694a7a7841 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AliasingEffects.ts @@ -50,7 +50,7 @@ export type AliasingEffect = /** * Mutate the value and any direct aliases (not captures). Errors if the value is not mutable. */ - | {kind: 'Mutate'; value: Place} + | {kind: 'Mutate'; value: Place; reason?: MutationReason | null} /** * Mutate the value and any direct aliases (not captures), but only if the value is known mutable. * This should be rare. @@ -174,6 +174,8 @@ export type AliasingEffect = place: Place; }; +export type MutationReason = {kind: 'AssignCurrentProperty'}; + export function hashEffect(effect: AliasingEffect): string { switch (effect.kind) { case 'Apply': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 2adf78fe058e..44bbaa463d69 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -68,7 +68,12 @@ import {FunctionSignature} from '../HIR/ObjectShape'; import {getWriteErrorReason} from './InferFunctionEffects'; import prettyFormat from 'pretty-format'; import {createTemporaryPlace} from '../HIR/HIRBuilder'; -import {AliasingEffect, AliasingSignature, hashEffect} from './AliasingEffects'; +import { + AliasingEffect, + AliasingSignature, + hashEffect, + MutationReason, +} from './AliasingEffects'; const DEBUG = false; @@ -452,18 +457,29 @@ function applySignature( effect.value.identifier.name.kind === 'named' ? `\`${effect.value.identifier.name.value}\`` : 'value'; + const diagnostic = CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: 'This value cannot be modified', + description: `${reason}.`, + }).withDetail({ + kind: 'error', + loc: effect.value.loc, + message: `${variable} cannot be modified`, + }); + if ( + effect.kind === 'Mutate' && + effect.reason?.kind === 'AssignCurrentProperty' + ) { + diagnostic.withDetail({ + kind: 'error', + loc: effect.value.loc, + message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`, + }); + } effects.push({ kind: 'MutateFrozen', place: effect.value, - error: CompilerDiagnostic.create({ - severity: ErrorSeverity.InvalidReact, - category: 'This value cannot be modified', - description: `${reason}.`, - }).withDetail({ - kind: 'error', - loc: effect.value.loc, - message: `${variable} cannot be modified`, - }), + error: diagnostic, }); } } @@ -1066,6 +1082,25 @@ function applyEffect( effect.value.identifier.name.kind === 'named' ? `\`${effect.value.identifier.name.value}\`` : 'value'; + const diagnostic = CompilerDiagnostic.create({ + severity: ErrorSeverity.InvalidReact, + category: 'This value cannot be modified', + description: `${reason}.`, + }).withDetail({ + kind: 'error', + loc: effect.value.loc, + message: `${variable} cannot be modified`, + }); + if ( + effect.kind === 'Mutate' && + effect.reason?.kind === 'AssignCurrentProperty' + ) { + diagnostic.withDetail({ + kind: 'error', + loc: effect.value.loc, + message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`, + }); + } applyEffect( context, state, @@ -1075,15 +1110,7 @@ function applyEffect( ? 'MutateFrozen' : 'MutateGlobal', place: effect.value, - error: CompilerDiagnostic.create({ - severity: ErrorSeverity.InvalidReact, - category: 'This value cannot be modified', - description: `${reason}.`, - }).withDetail({ - kind: 'error', - loc: effect.value.loc, - message: `${variable} cannot be modified`, - }), + error: diagnostic, }, initialized, effects, @@ -1680,7 +1707,15 @@ function computeSignatureForInstruction( } case 'PropertyStore': case 'ComputedStore': { - effects.push({kind: 'Mutate', value: value.object}); + const mutationReason: MutationReason | null = + value.kind === 'PropertyStore' && value.property === 'current' + ? {kind: 'AssignCurrentProperty'} + : null; + effects.push({ + kind: 'Mutate', + value: value.object, + reason: mutationReason, + }); effects.push({ kind: 'Capture', from: value.value, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md new file mode 100644 index 000000000000..af94ba06973f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +// @flow + +component Foo() { + const foo = useFoo(); + foo.current = true; + return
; +} + +``` + + +## Error + +``` +Found 1 error: + +Error: This value cannot be modified + +Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed. + + 3 | component Foo() { + 4 | const foo = useFoo(); +> 5 | foo.current = true; + | ^^^ value cannot be modified + 6 | return
; + 7 | } + 8 | + + 3 | component Foo() { + 4 | const foo = useFoo(); +> 5 | foo.current = true; + | ^^^ Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref". + 6 | return
; + 7 | } + 8 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js new file mode 100644 index 000000000000..efe92bc034e7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.js @@ -0,0 +1,7 @@ +// @flow + +component Foo() { + const foo = useFoo(); + foo.current = true; + return
; +} From 6ffcac8558efbd204c7df5d52787a90e507dd8d7 Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 15 Aug 2025 15:09:27 -0700 Subject: [PATCH 2/5] [compiler] Add support for diagnostic hints (#34126) Hints are meant as additional information to present to the developer about an error. The first use-case here is for the suggestion to name refs with "-Ref" if we encounter a mutation that looks like it might be a ref. The original error printing used a second error detail which printed the source code twice, a hint with just extra text is less noisy. --- .../src/CompilerError.ts | 28 ++++++++++++++----- .../Inference/InferMutationAliasingEffects.ts | 6 ++-- ...-assing-to-ref-current-in-render.expect.md | 8 +----- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index ee0d0f74c5f3..2055b25c0c94 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -58,11 +58,15 @@ export type CompilerDiagnosticDetail = /** * A/the source of the error */ - { - kind: 'error'; - loc: SourceLocation | null; - message: string; - }; + | { + kind: 'error'; + loc: SourceLocation | null; + message: string; + } + | { + kind: 'hint'; + message: string; + }; export enum CompilerSuggestionOperation { InsertBefore, @@ -134,7 +138,12 @@ export class CompilerDiagnostic { } primaryLocation(): SourceLocation | null { - return this.options.details.filter(d => d.kind === 'error')[0]?.loc ?? null; + const firstErrorDetail = this.options.details.filter( + d => d.kind === 'error', + )[0]; + return firstErrorDetail != null && firstErrorDetail.kind === 'error' + ? firstErrorDetail.loc + : null; } printErrorMessage(source: string, options: PrintErrorMessageOptions): string { @@ -167,9 +176,14 @@ export class CompilerDiagnostic { buffer.push(codeFrame); break; } + case 'hint': { + buffer.push('\n\n'); + buffer.push(detail.message); + break; + } default: { assertExhaustive( - detail.kind, + detail, `Unexpected detail kind ${(detail as any).kind}`, ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 44bbaa463d69..871c8296c35c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -471,8 +471,7 @@ function applySignature( effect.reason?.kind === 'AssignCurrentProperty' ) { diagnostic.withDetail({ - kind: 'error', - loc: effect.value.loc, + kind: 'hint', message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`, }); } @@ -1096,8 +1095,7 @@ function applyEffect( effect.reason?.kind === 'AssignCurrentProperty' ) { diagnostic.withDetail({ - kind: 'error', - loc: effect.value.loc, + kind: 'hint', message: `Hint: If this value is a Ref (value returned by \`useRef()\`), rename the variable to end in "Ref".`, }); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md index af94ba06973f..aef40d34cb5f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-assing-to-ref-current-in-render.expect.md @@ -30,13 +30,7 @@ Modifying a value returned from a hook is not allowed. Consider moving the modif 7 | } 8 | - 3 | component Foo() { - 4 | const foo = useFoo(); -> 5 | foo.current = true; - | ^^^ Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref". - 6 | return
; - 7 | } - 8 | +Hint: If this value is a Ref (value returned by `useRef()`), rename the variable to end in "Ref". ``` \ No newline at end of file From eaf6adb1277e4cb4f91d1b7f687f773657a5751b Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 15 Aug 2025 15:21:28 -0700 Subject: [PATCH 3/5] [compiler][wip] Remove old mutation/aliasing implementation (#34028) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34028). * #34029 * __->__ #34028 --- .../src/Entrypoint/Pipeline.ts | 55 +- .../src/HIR/Environment.ts | 5 - .../src/Inference/AnalyseFunctions.ts | 74 +- .../InerAliasForUncalledFunctions.ts | 134 -- .../src/Inference/InferAlias.ts | 68 - .../src/Inference/InferAliasForPhis.ts | 27 - .../src/Inference/InferAliasForStores.ts | 68 - .../src/Inference/InferFunctionEffects.ts | 351 --- .../src/Inference/InferMutableLifetimes.ts | 218 -- .../src/Inference/InferMutableRanges.ts | 102 - .../Inference/InferMutableRangesForAlias.ts | 54 - .../Inference/InferMutationAliasingEffects.ts | 204 +- .../src/Inference/InferReferenceEffects.ts | 2125 ----------------- .../src/Inference/InferTryCatchAliases.ts | 49 - .../src/Inference/index.ts | 2 - .../ReactiveScopes/PruneNonEscapingScopes.ts | 2 +- .../ValidateLocalsNotReassignedAfterRender.ts | 2 +- ...ValidateNoFreezingKnownMutableFunctions.ts | 5 +- .../ValidateNoImpureFunctionsInRender.ts | 2 +- ...global-in-function-spread-as-jsx.expect.md | 39 + ...ng-to-global-in-function-spread-as-jsx.js} | 1 + ...g-aliased-capture-aliased-mutate.expect.md | 107 - .../bug-aliased-capture-aliased-mutate.js | 53 - .../bug-aliased-capture-mutate.expect.md | 87 - .../compiler/bug-aliased-capture-mutate.js | 34 - ...-func-maybealias-captured-mutate.expect.md | 22 +- .../bug-invalid-phi-as-dependency.expect.md | 92 - .../bug-invalid-phi-as-dependency.tsx | 31 - ...nstruction-hoisted-sequence-expr.expect.md | 110 - ...fter-construction-hoisted-sequence-expr.js | 32 - ...n-global-in-jsx-spread-attribute.expect.md | 33 - ...ive-ref-validation-in-use-effect.expect.md | 72 - ...e-positive-ref-validation-in-use-effect.js | 27 - ...r.object-capture-global-mutation.expect.md | 39 - ...es-function-with-global-mutation.expect.md | 49 + ...captures-function-with-global-mutation.js} | 1 - 36 files changed, 311 insertions(+), 4065 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InerAliasForUncalledFunctions.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForPhis.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForStores.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForAlias.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferTryCatchAliases.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.assign-global-in-jsx-spread-attribute.js => allow-assigning-to-global-in-function-spread-as-jsx.js} (61%) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.js delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.object-capture-global-mutation.js => object-expression-captures-function-with-global-mutation.js} (81%) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index a4f984f1958d..e5005d02c4ef 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -33,9 +33,7 @@ import {findContextIdentifiers} from '../HIR/FindContextIdentifiers'; import { analyseFunctions, dropManualMemoization, - inferMutableRanges, inferReactivePlaces, - inferReferenceEffects, inlineImmediatelyInvokedFunctionExpressions, inferEffectDependencies, } from '../Inference'; @@ -100,7 +98,6 @@ import {outlineJSX} from '../Optimization/OutlineJsx'; import {optimizePropsMethodCalls} from '../Optimization/OptimizePropsMethodCalls'; import {transformFire} from '../Transform'; import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender'; -import {CompilerError} from '..'; import {validateStaticComponents} from '../Validation/ValidateStaticComponents'; import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions'; import {inferMutationAliasingEffects} from '../Inference/InferMutationAliasingEffects'; @@ -229,28 +226,14 @@ function runWithEnvironment( analyseFunctions(hir); log({kind: 'hir', name: 'AnalyseFunctions', value: hir}); - if (!env.config.enableNewMutationAliasingModel) { - const fnEffectErrors = inferReferenceEffects(hir); - if (env.isInferredMemoEnabled) { - if (fnEffectErrors.length > 0) { - CompilerError.throw(fnEffectErrors[0]); - } - } - log({kind: 'hir', name: 'InferReferenceEffects', value: hir}); - } else { - const mutabilityAliasingErrors = inferMutationAliasingEffects(hir); - log({kind: 'hir', name: 'InferMutationAliasingEffects', value: hir}); - if (env.isInferredMemoEnabled) { - if (mutabilityAliasingErrors.isErr()) { - throw mutabilityAliasingErrors.unwrapErr(); - } + const mutabilityAliasingErrors = inferMutationAliasingEffects(hir); + log({kind: 'hir', name: 'InferMutationAliasingEffects', value: hir}); + if (env.isInferredMemoEnabled) { + if (mutabilityAliasingErrors.isErr()) { + throw mutabilityAliasingErrors.unwrapErr(); } } - if (!env.config.enableNewMutationAliasingModel) { - validateLocalsNotReassignedAfterRender(hir); - } - // Note: Has to come after infer reference effects because "dead" code may still affect inference deadCodeElimination(hir); log({kind: 'hir', name: 'DeadCodeElimination', value: hir}); @@ -263,20 +246,15 @@ function runWithEnvironment( pruneMaybeThrows(hir); log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); - if (!env.config.enableNewMutationAliasingModel) { - inferMutableRanges(hir); - log({kind: 'hir', name: 'InferMutableRanges', value: hir}); - } else { - const mutabilityAliasingErrors = inferMutationAliasingRanges(hir, { - isFunctionExpression: false, - }); - log({kind: 'hir', name: 'InferMutationAliasingRanges', value: hir}); - if (env.isInferredMemoEnabled) { - if (mutabilityAliasingErrors.isErr()) { - throw mutabilityAliasingErrors.unwrapErr(); - } - validateLocalsNotReassignedAfterRender(hir); + const mutabilityAliasingRangeErrors = inferMutationAliasingRanges(hir, { + isFunctionExpression: false, + }); + log({kind: 'hir', name: 'InferMutationAliasingRanges', value: hir}); + if (env.isInferredMemoEnabled) { + if (mutabilityAliasingRangeErrors.isErr()) { + throw mutabilityAliasingRangeErrors.unwrapErr(); } + validateLocalsNotReassignedAfterRender(hir); } if (env.isInferredMemoEnabled) { @@ -308,12 +286,7 @@ function runWithEnvironment( validateNoImpureFunctionsInRender(hir).unwrap(); } - if ( - env.config.validateNoFreezingKnownMutableFunctions || - env.config.enableNewMutationAliasingModel - ) { - validateNoFreezingKnownMutableFunctions(hir).unwrap(); - } + validateNoFreezingKnownMutableFunctions(hir).unwrap(); } inferReactivePlaces(hir); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 957c5ab84ab2..fd68830f929d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -250,11 +250,6 @@ export const EnvironmentConfigSchema = z.object({ */ flowTypeProvider: z.nullable(z.function().args(z.string())).default(null), - /** - * Enable a new model for mutability and aliasing inference - */ - enableNewMutationAliasingModel: z.boolean().default(true), - /** * Enables inference of optional dependency chains. Without this flag * a property chain such as `props?.items?.foo` will infer as a dep on diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index 78d82c0c461d..77a2bdcde596 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -6,20 +6,10 @@ */ import {CompilerError} from '../CompilerError'; -import { - Effect, - HIRFunction, - Identifier, - IdentifierId, - LoweredFunction, - isRefOrRefValue, - makeInstructionId, -} from '../HIR'; +import {Effect, HIRFunction, IdentifierId, makeInstructionId} from '../HIR'; import {deadCodeElimination} from '../Optimization'; import {inferReactiveScopeVariables} from '../ReactiveScopes'; import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; -import {inferMutableRanges} from './InferMutableRanges'; -import inferReferenceEffects from './InferReferenceEffects'; import {assertExhaustive} from '../Utils/utils'; import {inferMutationAliasingEffects} from './InferMutationAliasingEffects'; import {inferMutationAliasingRanges} from './InferMutationAliasingRanges'; @@ -30,12 +20,7 @@ export default function analyseFunctions(func: HIRFunction): void { switch (instr.value.kind) { case 'ObjectMethod': case 'FunctionExpression': { - if (!func.env.config.enableNewMutationAliasingModel) { - lower(instr.value.loweredFunc.func); - infer(instr.value.loweredFunc); - } else { - lowerWithMutationAliasing(instr.value.loweredFunc.func); - } + lowerWithMutationAliasing(instr.value.loweredFunc.func); /** * Reset mutable range for outer inferReferenceEffects @@ -140,58 +125,3 @@ function lowerWithMutationAliasing(fn: HIRFunction): void { value: fn, }); } - -function lower(func: HIRFunction): void { - analyseFunctions(func); - inferReferenceEffects(func, {isFunctionExpression: true}); - deadCodeElimination(func); - inferMutableRanges(func); - rewriteInstructionKindsBasedOnReassignment(func); - inferReactiveScopeVariables(func); - func.env.logger?.debugLogIRs?.({ - kind: 'hir', - name: 'AnalyseFunction (inner)', - value: func, - }); -} - -function infer(loweredFunc: LoweredFunction): void { - for (const operand of loweredFunc.func.context) { - const identifier = operand.identifier; - CompilerError.invariant(operand.effect === Effect.Unknown, { - reason: - '[AnalyseFunctions] Expected Function context effects to not have been set', - loc: operand.loc, - }); - if (isRefOrRefValue(identifier)) { - /* - * TODO: this is a hack to ensure we treat functions which reference refs - * as having a capture and therefore being considered mutable. this ensures - * the function gets a mutable range which accounts for anywhere that it - * could be called, and allows us to help ensure it isn't called during - * render - */ - operand.effect = Effect.Capture; - } else if (isMutatedOrReassigned(identifier)) { - /** - * Reflects direct reassignments, PropertyStores, and ConditionallyMutate - * (directly or through maybe-aliases) - */ - operand.effect = Effect.Capture; - } else { - operand.effect = Effect.Read; - } - } -} - -function isMutatedOrReassigned(id: Identifier): boolean { - /* - * This check checks for mutation and reassingnment, so the usual check for - * mutation (ie, `mutableRange.end - mutableRange.start > 1`) isn't quite - * enough. - * - * We need to track re-assignments in context refs as we need to reflect the - * re-assignment back to the captured refs. - */ - return id.mutableRange.end > id.mutableRange.start; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InerAliasForUncalledFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InerAliasForUncalledFunctions.ts deleted file mode 100644 index 1dc5743b7370..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InerAliasForUncalledFunctions.ts +++ /dev/null @@ -1,134 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - Effect, - HIRFunction, - Identifier, - isMutableEffect, - isRefOrRefLikeMutableType, - makeInstructionId, -} from '../HIR/HIR'; -import {eachInstructionValueOperand} from '../HIR/visitors'; -import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; -import DisjointSet from '../Utils/DisjointSet'; - -/** - * If a function captures a mutable value but never gets called, we don't infer a - * mutable range for that function. This means that we also don't alias the function - * with its mutable captures. - * - * This case is tricky, because we don't generally know for sure what is a mutation - * and what may just be a normal function call. For example: - * - * ``` - * hook useFoo() { - * const x = makeObject(); - * return () => { - * return readObject(x); // could be a mutation! - * } - * } - * ``` - * - * If we pessimistically assume that all such cases are mutations, we'd have to group - * lots of memo scopes together unnecessarily. However, if there is definitely a mutation: - * - * ``` - * hook useFoo(createEntryForKey) { - * const cache = new WeakMap(); - * return (key) => { - * let entry = cache.get(key); - * if (entry == null) { - * entry = createEntryForKey(key); - * cache.set(key, entry); // known mutation! - * } - * return entry; - * } - * } - * ``` - * - * Then we have to ensure that the function and its mutable captures alias together and - * end up in the same scope. However, aliasing together isn't enough if the function - * and operands all have empty mutable ranges (end = start + 1). - * - * This pass finds function expressions and object methods that have an empty mutable range - * and known-mutable operands which also don't have a mutable range, and ensures that the - * function and those operands are aliased together *and* that their ranges are updated to - * end after the function expression. This is sufficient to ensure that a reactive scope is - * created for the alias set. - */ -export function inferAliasForUncalledFunctions( - fn: HIRFunction, - aliases: DisjointSet, -): void { - for (const block of fn.body.blocks.values()) { - instrs: for (const instr of block.instructions) { - const {lvalue, value} = instr; - if ( - value.kind !== 'ObjectMethod' && - value.kind !== 'FunctionExpression' - ) { - continue; - } - /* - * If the function is known to be mutated, we will have - * already aliased any mutable operands with it - */ - const range = lvalue.identifier.mutableRange; - if (range.end > range.start + 1) { - continue; - } - /* - * If the function already has operands with an active mutable range, - * then we don't need to do anything — the function will have already - * been visited and included in some mutable alias set. This case can - * also occur due to visiting the same function in an earlier iteration - * of the outer fixpoint loop. - */ - for (const operand of eachInstructionValueOperand(value)) { - if (isMutable(instr, operand)) { - continue instrs; - } - } - const operands: Set = new Set(); - for (const effect of value.loweredFunc.func.effects ?? []) { - if (effect.kind !== 'ContextMutation') { - continue; - } - /* - * We're looking for known-mutations only, so we look at the effects - * rather than function context - */ - if (effect.effect === Effect.Store || effect.effect === Effect.Mutate) { - for (const operand of effect.places) { - /* - * It's possible that function effect analysis thinks there was a context mutation, - * but then InferReferenceEffects figures out some operands are globals and therefore - * creates a non-mutable effect for those operands. - * We should change InferReferenceEffects to swap the ContextMutation for a global - * mutation in that case, but for now we just filter them out here - */ - if ( - isMutableEffect(operand.effect, operand.loc) && - !isRefOrRefLikeMutableType(operand.identifier.type) - ) { - operands.add(operand.identifier); - } - } - } - } - if (operands.size !== 0) { - operands.add(lvalue.identifier); - aliases.union([...operands]); - // Update mutable ranges, if the ranges are empty then a reactive scope isn't created - for (const operand of operands) { - operand.mutableRange.end = makeInstructionId(instr.id + 1); - } - } - } - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts deleted file mode 100644 index 80422c8391f4..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAlias.ts +++ /dev/null @@ -1,68 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - HIRFunction, - Identifier, - Instruction, - isPrimitiveType, - Place, -} from '../HIR/HIR'; -import DisjointSet from '../Utils/DisjointSet'; - -export type AliasSet = Set; - -export function inferAliases(func: HIRFunction): DisjointSet { - const aliases = new DisjointSet(); - for (const [_, block] of func.body.blocks) { - for (const instr of block.instructions) { - inferInstr(instr, aliases); - } - } - - return aliases; -} - -function inferInstr( - instr: Instruction, - aliases: DisjointSet, -): void { - const {lvalue, value: instrValue} = instr; - let alias: Place | null = null; - switch (instrValue.kind) { - case 'LoadLocal': - case 'LoadContext': { - if (isPrimitiveType(instrValue.place.identifier)) { - return; - } - alias = instrValue.place; - break; - } - case 'StoreLocal': - case 'StoreContext': { - alias = instrValue.value; - break; - } - case 'Destructure': { - alias = instrValue.value; - break; - } - case 'ComputedLoad': - case 'PropertyLoad': { - alias = instrValue.object; - break; - } - case 'TypeCastExpression': { - alias = instrValue.value; - break; - } - default: - return; - } - - aliases.union([lvalue.identifier, alias.identifier]); -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForPhis.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForPhis.ts deleted file mode 100644 index e81e3ebdae7a..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForPhis.ts +++ /dev/null @@ -1,27 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {HIRFunction, Identifier} from '../HIR/HIR'; -import DisjointSet from '../Utils/DisjointSet'; - -export function inferAliasForPhis( - func: HIRFunction, - aliases: DisjointSet, -): void { - for (const [_, block] of func.body.blocks) { - for (const phi of block.phis) { - const isPhiMutatedAfterCreation: boolean = - phi.place.identifier.mutableRange.end > - (block.instructions.at(0)?.id ?? block.terminal.id); - if (isPhiMutatedAfterCreation) { - for (const [, operand] of phi.operands) { - aliases.union([phi.place.identifier, operand.identifier]); - } - } - } - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForStores.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForStores.ts deleted file mode 100644 index d8d17f6a5fa0..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferAliasForStores.ts +++ /dev/null @@ -1,68 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - Effect, - HIRFunction, - Identifier, - InstructionId, - Place, -} from '../HIR/HIR'; -import { - eachInstructionLValue, - eachInstructionValueOperand, -} from '../HIR/visitors'; -import DisjointSet from '../Utils/DisjointSet'; - -export function inferAliasForStores( - func: HIRFunction, - aliases: DisjointSet, -): void { - for (const [_, block] of func.body.blocks) { - for (const instr of block.instructions) { - const {value, lvalue} = instr; - const isStore = - lvalue.effect === Effect.Store || - /* - * Some typed functions annotate callees or arguments - * as Effect.Store. - */ - ![...eachInstructionValueOperand(value)].every( - operand => operand.effect !== Effect.Store, - ); - - if (!isStore) { - continue; - } - for (const operand of eachInstructionLValue(instr)) { - maybeAlias(aliases, lvalue, operand, instr.id); - } - for (const operand of eachInstructionValueOperand(value)) { - if ( - operand.effect === Effect.Capture || - operand.effect === Effect.Store - ) { - maybeAlias(aliases, lvalue, operand, instr.id); - } - } - } - } -} - -function maybeAlias( - aliases: DisjointSet, - lvalue: Place, - rvalue: Place, - id: InstructionId, -): void { - if ( - lvalue.identifier.mutableRange.end > id + 1 || - rvalue.identifier.mutableRange.end > id - ) { - aliases.union([lvalue.identifier, rvalue.identifier]); - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts deleted file mode 100644 index a01ca188a0af..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferFunctionEffects.ts +++ /dev/null @@ -1,351 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - CompilerError, - CompilerErrorDetailOptions, - ErrorSeverity, - ValueKind, -} from '..'; -import { - AbstractValue, - BasicBlock, - Effect, - Environment, - FunctionEffect, - Instruction, - InstructionValue, - Place, - ValueReason, - getHookKind, - isRefOrRefValue, -} from '../HIR'; -import {eachInstructionOperand, eachTerminalOperand} from '../HIR/visitors'; -import {assertExhaustive} from '../Utils/utils'; - -interface State { - kind(place: Place): AbstractValue; - values(place: Place): Array; - isDefined(place: Place): boolean; -} - -function inferOperandEffect(state: State, place: Place): null | FunctionEffect { - const value = state.kind(place); - CompilerError.invariant(value != null, { - reason: 'Expected operand to have a kind', - loc: null, - }); - - switch (place.effect) { - case Effect.Store: - case Effect.Mutate: { - if (isRefOrRefValue(place.identifier)) { - break; - } else if (value.kind === ValueKind.Context) { - CompilerError.invariant(value.context.size > 0, { - reason: - "[InferFunctionEffects] Expected Context-kind value's capture list to be non-empty.", - loc: place.loc, - }); - return { - kind: 'ContextMutation', - loc: place.loc, - effect: place.effect, - places: value.context, - }; - } else if ( - value.kind !== ValueKind.Mutable && - // We ignore mutations of primitives since this is not a React-specific problem - value.kind !== ValueKind.Primitive - ) { - let reason = getWriteErrorReason(value); - return { - kind: - value.reason.size === 1 && value.reason.has(ValueReason.Global) - ? 'GlobalMutation' - : 'ReactMutation', - error: { - reason, - description: - place.identifier.name !== null && - place.identifier.name.kind === 'named' - ? `Found mutation of \`${place.identifier.name.value}\`` - : null, - loc: place.loc, - suggestions: null, - severity: ErrorSeverity.InvalidReact, - }, - }; - } - break; - } - } - return null; -} - -function inheritFunctionEffects( - state: State, - place: Place, -): Array { - const effects = inferFunctionInstrEffects(state, place); - - return effects - .flatMap(effect => { - if (effect.kind === 'GlobalMutation' || effect.kind === 'ReactMutation') { - return [effect]; - } else { - const effects: Array = []; - CompilerError.invariant(effect.kind === 'ContextMutation', { - reason: 'Expected ContextMutation', - loc: null, - }); - /** - * Contextual effects need to be replayed against the current inference - * state, which may know more about the value to which the effect applied. - * The main cases are: - * 1. The mutated context value is _still_ a context value in the current scope, - * so we have to continue propagating the original context mutation. - * 2. The mutated context value is a mutable value in the current scope, - * so the context mutation was fine and we can skip propagating the effect. - * 3. The mutated context value is an immutable value in the current scope, - * resulting in a non-ContextMutation FunctionEffect. We propagate that new, - * more detailed effect to the current function context. - */ - for (const place of effect.places) { - if (state.isDefined(place)) { - const replayedEffect = inferOperandEffect(state, { - ...place, - loc: effect.loc, - effect: effect.effect, - }); - if (replayedEffect != null) { - if (replayedEffect.kind === 'ContextMutation') { - // Case 1, still a context value so propagate the original effect - effects.push(effect); - } else { - // Case 3, immutable value so propagate the more precise effect - effects.push(replayedEffect); - } - } // else case 2, local mutable value so this effect was fine - } - } - return effects; - } - }) - .filter((effect): effect is FunctionEffect => effect != null); -} - -function inferFunctionInstrEffects( - state: State, - place: Place, -): Array { - const effects: Array = []; - const instrs = state.values(place); - CompilerError.invariant(instrs != null, { - reason: 'Expected operand to have instructions', - loc: null, - }); - - for (const instr of instrs) { - if ( - (instr.kind === 'FunctionExpression' || instr.kind === 'ObjectMethod') && - instr.loweredFunc.func.effects != null - ) { - effects.push(...instr.loweredFunc.func.effects); - } - } - - return effects; -} - -function operandEffects( - state: State, - place: Place, - filterRenderSafe: boolean, -): Array { - const functionEffects: Array = []; - const effect = inferOperandEffect(state, place); - effect && functionEffects.push(effect); - functionEffects.push(...inheritFunctionEffects(state, place)); - if (filterRenderSafe) { - return functionEffects.filter(effect => !isEffectSafeOutsideRender(effect)); - } else { - return functionEffects; - } -} - -export function inferInstructionFunctionEffects( - env: Environment, - state: State, - instr: Instruction, -): Array { - const functionEffects: Array = []; - switch (instr.value.kind) { - case 'JsxExpression': { - if (instr.value.tag.kind === 'Identifier') { - functionEffects.push(...operandEffects(state, instr.value.tag, false)); - } - instr.value.children?.forEach(child => - functionEffects.push(...operandEffects(state, child, false)), - ); - for (const attr of instr.value.props) { - if (attr.kind === 'JsxSpreadAttribute') { - functionEffects.push(...operandEffects(state, attr.argument, false)); - } else { - functionEffects.push(...operandEffects(state, attr.place, true)); - } - } - break; - } - case 'ObjectMethod': - case 'FunctionExpression': { - /** - * If this function references other functions, propagate the referenced function's - * effects to this function. - * - * ``` - * let f = () => global = true; - * let g = () => f(); - * g(); - * ``` - * - * In this example, because `g` references `f`, we propagate the GlobalMutation from - * `f` to `g`. Thus, referencing `g` in `g()` will evaluate the GlobalMutation in the outer - * function effect context and report an error. But if instead we do: - * - * ``` - * let f = () => global = true; - * let g = () => f(); - * useEffect(() => g(), [g]) - * ``` - * - * Now `g`'s effects will be discarded since they're in a useEffect. - */ - for (const operand of eachInstructionOperand(instr)) { - instr.value.loweredFunc.func.effects ??= []; - instr.value.loweredFunc.func.effects.push( - ...inferFunctionInstrEffects(state, operand), - ); - } - break; - } - case 'MethodCall': - case 'CallExpression': { - let callee; - if (instr.value.kind === 'MethodCall') { - callee = instr.value.property; - functionEffects.push( - ...operandEffects(state, instr.value.receiver, false), - ); - } else { - callee = instr.value.callee; - } - functionEffects.push(...operandEffects(state, callee, false)); - let isHook = getHookKind(env, callee.identifier) != null; - for (const arg of instr.value.args) { - const place = arg.kind === 'Identifier' ? arg : arg.place; - /* - * Join the effects of the argument with the effects of the enclosing function, - * unless the we're detecting a global mutation inside a useEffect hook - */ - functionEffects.push(...operandEffects(state, place, isHook)); - } - break; - } - case 'StartMemoize': - case 'FinishMemoize': - case 'LoadLocal': - case 'StoreLocal': { - break; - } - case 'StoreGlobal': { - functionEffects.push({ - kind: 'GlobalMutation', - error: { - reason: - 'Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render)', - loc: instr.loc, - suggestions: null, - severity: ErrorSeverity.InvalidReact, - }, - }); - break; - } - default: { - for (const operand of eachInstructionOperand(instr)) { - functionEffects.push(...operandEffects(state, operand, false)); - } - } - } - return functionEffects; -} - -export function inferTerminalFunctionEffects( - state: State, - block: BasicBlock, -): Array { - const functionEffects: Array = []; - for (const operand of eachTerminalOperand(block.terminal)) { - functionEffects.push(...operandEffects(state, operand, true)); - } - return functionEffects; -} - -export function transformFunctionEffectErrors( - functionEffects: Array, -): Array { - return functionEffects.map(eff => { - switch (eff.kind) { - case 'ReactMutation': - case 'GlobalMutation': { - return eff.error; - } - case 'ContextMutation': { - return { - severity: ErrorSeverity.Invariant, - reason: `Unexpected ContextMutation in top-level function effects`, - loc: eff.loc, - }; - } - default: - assertExhaustive( - eff, - `Unexpected function effect kind \`${(eff as any).kind}\``, - ); - } - }); -} - -function isEffectSafeOutsideRender(effect: FunctionEffect): boolean { - return effect.kind === 'GlobalMutation'; -} - -export function getWriteErrorReason(abstractValue: AbstractValue): string { - if (abstractValue.reason.has(ValueReason.Global)) { - return 'Modifying a variable defined outside a component or hook is not allowed. Consider using an effect'; - } else if (abstractValue.reason.has(ValueReason.JsxCaptured)) { - return 'Modifying a value used previously in JSX is not allowed. Consider moving the modification before the JSX'; - } else if (abstractValue.reason.has(ValueReason.Context)) { - return `Modifying a value returned from 'useContext()' is not allowed.`; - } else if (abstractValue.reason.has(ValueReason.KnownReturnSignature)) { - return 'Modifying a value returned from a function whose return value should not be mutated'; - } else if (abstractValue.reason.has(ValueReason.ReactiveFunctionArgument)) { - return 'Modifying component props or hook arguments is not allowed. Consider using a local variable instead'; - } else if (abstractValue.reason.has(ValueReason.State)) { - return "Modifying a value returned from 'useState()', which should not be modified directly. Use the setter function to update instead"; - } else if (abstractValue.reason.has(ValueReason.ReducerState)) { - return "Modifying a value returned from 'useReducer()', which should not be modified directly. Use the dispatch function to update instead"; - } else if (abstractValue.reason.has(ValueReason.Effect)) { - return 'Modifying a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the modification before calling useEffect()'; - } else if (abstractValue.reason.has(ValueReason.HookCaptured)) { - return 'Modifying a value previously passed as an argument to a hook is not allowed. Consider moving the modification before calling the hook'; - } else if (abstractValue.reason.has(ValueReason.HookReturn)) { - return 'Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed'; - } else { - return 'This modifies a variable that React considers immutable'; - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts deleted file mode 100644 index 5057a7ac88ca..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableLifetimes.ts +++ /dev/null @@ -1,218 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - Effect, - HIRFunction, - Identifier, - InstructionId, - InstructionKind, - isArrayType, - isMapType, - isRefOrRefValue, - isSetType, - makeInstructionId, - Place, -} from '../HIR/HIR'; -import {printPlace} from '../HIR/PrintHIR'; -import { - eachInstructionLValue, - eachInstructionOperand, - eachTerminalOperand, -} from '../HIR/visitors'; -import {assertExhaustive} from '../Utils/utils'; - -/* - * For each usage of a value in the given function, determines if the usage - * may be succeeded by a mutable usage of that same value and if so updates - * the usage to be mutable. - * - * Stated differently, this inference ensures that inferred capabilities of - * each reference are as follows: - * - freeze: the value is frozen at this point - * - readonly: the value is not modified at this point *or any subsequent - * point* - * - mutable: the value is modified at this point *or some subsequent point*. - * - * Note that this refines the capabilities inferered by InferReferenceCapability, - * which looks at individual references and not the lifetime of a value's mutability. - * - * == Algorithm - * - * TODO: - * 1. Forward data-flow analysis to determine aliasing. Unlike InferReferenceCapability - * which only tracks aliasing of top-level variables (`y = x`), this analysis needs - * to know if a value is aliased anywhere (`y.x = x`). The forward data flow tracks - * all possible locations which may have aliased a value. The concrete result is - * a mapping of each Place to the set of possibly-mutable values it may alias. - * - * ``` - * const x = []; // {x: v0; v0: mutable []} - * const y = {}; // {x: v0, y: v1; v0: mutable [], v1: mutable []} - * y.x = x; // {x: v0, y: v1; v0: mutable [v1], v1: mutable [v0]} - * read(x); // {x: v0, y: v1; v0: mutable [v1], v1: mutable [v0]} - * mutate(y); // can infer that y mutates v0 and v1 - * ``` - * - * DONE: - * 2. Forward data-flow analysis to compute mutability liveness. Walk forwards over - * the CFG and track which values are mutated in a successor. - * - * ``` - * mutate(y); // mutable y => v0, v1 mutated - * read(x); // x maps to v0, v1, those are in the mutated-later set, so x is mutable here - * ... - * ``` - */ - -function infer(place: Place, instrId: InstructionId): void { - if (!isRefOrRefValue(place.identifier)) { - place.identifier.mutableRange.end = makeInstructionId(instrId + 1); - } -} - -function inferPlace( - place: Place, - instrId: InstructionId, - inferMutableRangeForStores: boolean, -): void { - switch (place.effect) { - case Effect.Unknown: { - throw new Error(`Found an unknown place ${printPlace(place)}}!`); - } - case Effect.Capture: - case Effect.Read: - case Effect.Freeze: - return; - case Effect.Store: - if (inferMutableRangeForStores) { - infer(place, instrId); - } - return; - case Effect.ConditionallyMutateIterator: { - const identifier = place.identifier; - if ( - !isArrayType(identifier) && - !isSetType(identifier) && - !isMapType(identifier) - ) { - infer(place, instrId); - } - return; - } - case Effect.ConditionallyMutate: - case Effect.Mutate: { - infer(place, instrId); - return; - } - default: - assertExhaustive(place.effect, `Unexpected ${printPlace(place)} effect`); - } -} - -export function inferMutableLifetimes( - func: HIRFunction, - inferMutableRangeForStores: boolean, -): void { - /* - * Context variables only appear to mutate where they are assigned, but we need - * to force their range to start at their declaration. Track the declaring instruction - * id so that the ranges can be extended if/when they are reassigned - */ - const contextVariableDeclarationInstructions = new Map< - Identifier, - InstructionId - >(); - for (const [_, block] of func.body.blocks) { - for (const phi of block.phis) { - const isPhiMutatedAfterCreation: boolean = - phi.place.identifier.mutableRange.end > - (block.instructions.at(0)?.id ?? block.terminal.id); - if ( - inferMutableRangeForStores && - isPhiMutatedAfterCreation && - phi.place.identifier.mutableRange.start === 0 - ) { - for (const [, operand] of phi.operands) { - if (phi.place.identifier.mutableRange.start === 0) { - phi.place.identifier.mutableRange.start = - operand.identifier.mutableRange.start; - } else { - phi.place.identifier.mutableRange.start = makeInstructionId( - Math.min( - phi.place.identifier.mutableRange.start, - operand.identifier.mutableRange.start, - ), - ); - } - } - } - } - - for (const instr of block.instructions) { - for (const operand of eachInstructionLValue(instr)) { - const lvalueId = operand.identifier; - - /* - * lvalue start being mutable when they're initially assigned a - * value. - */ - lvalueId.mutableRange.start = instr.id; - - /* - * Let's be optimistic and assume this lvalue is not mutable by - * default. - */ - lvalueId.mutableRange.end = makeInstructionId(instr.id + 1); - } - for (const operand of eachInstructionOperand(instr)) { - inferPlace(operand, instr.id, inferMutableRangeForStores); - } - - if ( - instr.value.kind === 'DeclareContext' || - (instr.value.kind === 'StoreContext' && - instr.value.lvalue.kind !== InstructionKind.Reassign && - !contextVariableDeclarationInstructions.has( - instr.value.lvalue.place.identifier, - )) - ) { - /** - * Save declarations of context variables if they hasn't already been - * declared (due to hoisted declarations). - */ - contextVariableDeclarationInstructions.set( - instr.value.lvalue.place.identifier, - instr.id, - ); - } else if (instr.value.kind === 'StoreContext') { - /* - * Else this is a reassignment, extend the range from the declaration (if present). - * Note that declarations may not be present for context variables that are reassigned - * within a function expression before (or without) a read of the same variable - */ - const declaration = contextVariableDeclarationInstructions.get( - instr.value.lvalue.place.identifier, - ); - if ( - declaration != null && - !isRefOrRefValue(instr.value.lvalue.place.identifier) - ) { - const range = instr.value.lvalue.place.identifier.mutableRange; - if (range.start === 0) { - range.start = declaration; - } else { - range.start = makeInstructionId(Math.min(range.start, declaration)); - } - } - } - } - for (const operand of eachTerminalOperand(block.terminal)) { - inferPlace(operand, block.terminal.id, inferMutableRangeForStores); - } - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts deleted file mode 100644 index 571a19290ea6..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts +++ /dev/null @@ -1,102 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {HIRFunction, Identifier} from '../HIR/HIR'; -import {inferAliasForUncalledFunctions} from './InerAliasForUncalledFunctions'; -import {inferAliases} from './InferAlias'; -import {inferAliasForPhis} from './InferAliasForPhis'; -import {inferAliasForStores} from './InferAliasForStores'; -import {inferMutableLifetimes} from './InferMutableLifetimes'; -import {inferMutableRangesForAlias} from './InferMutableRangesForAlias'; -import {inferTryCatchAliases} from './InferTryCatchAliases'; - -export function inferMutableRanges(ir: HIRFunction): void { - // Infer mutable ranges for non fields - inferMutableLifetimes(ir, false); - - // Calculate aliases - const aliases = inferAliases(ir); - /* - * Calculate aliases for try/catch, where any value created - * in the try block could be aliased to the catch param - */ - inferTryCatchAliases(ir, aliases); - - /* - * Eagerly canonicalize so that if nothing changes we can bail out - * after a single iteration - */ - let prevAliases: Map = aliases.canonicalize(); - while (true) { - // Infer mutable ranges for aliases that are not fields - inferMutableRangesForAlias(ir, aliases); - - // Update aliasing information of fields - inferAliasForStores(ir, aliases); - - // Update aliasing information of phis - inferAliasForPhis(ir, aliases); - - const nextAliases = aliases.canonicalize(); - if (areEqualMaps(prevAliases, nextAliases)) { - break; - } - prevAliases = nextAliases; - } - - // Re-infer mutable ranges for all values - inferMutableLifetimes(ir, true); - - /** - * The second inferMutableLifetimes() call updates mutable ranges - * of values to account for Store effects. Now we need to update - * all aliases of such values to extend their ranges as well. Note - * that the store only mutates the the directly aliased value and - * not any of its inner captured references. For example: - * - * ``` - * let y; - * if (cond) { - * y = []; - * } else { - * y = [{}]; - * } - * y.push(z); - * ``` - * - * The Store effect from the `y.push` modifies the values that `y` - * directly aliases - the two arrays from the if/else branches - - * but does not modify values that `y` "contains" such as the - * object literal or `z`. - */ - prevAliases = aliases.canonicalize(); - while (true) { - inferMutableRangesForAlias(ir, aliases); - inferAliasForPhis(ir, aliases); - inferAliasForUncalledFunctions(ir, aliases); - const nextAliases = aliases.canonicalize(); - if (areEqualMaps(prevAliases, nextAliases)) { - break; - } - prevAliases = nextAliases; - } -} - -function areEqualMaps(a: Map, b: Map): boolean { - if (a.size !== b.size) { - return false; - } - for (const [key, value] of a) { - if (!b.has(key)) { - return false; - } - if (b.get(key) !== value) { - return false; - } - } - return true; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForAlias.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForAlias.ts deleted file mode 100644 index a7e8b5c1f7a8..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForAlias.ts +++ /dev/null @@ -1,54 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - HIRFunction, - Identifier, - InstructionId, - isRefOrRefValue, -} from '../HIR/HIR'; -import DisjointSet from '../Utils/DisjointSet'; - -export function inferMutableRangesForAlias( - _fn: HIRFunction, - aliases: DisjointSet, -): void { - const aliasSets = aliases.buildSets(); - for (const aliasSet of aliasSets) { - /* - * Update mutableRange.end only if the identifiers have actually been - * mutated. - */ - const mutatingIdentifiers = [...aliasSet].filter( - id => - id.mutableRange.end - id.mutableRange.start > 1 && !isRefOrRefValue(id), - ); - - if (mutatingIdentifiers.length > 0) { - // Find final instruction which mutates this alias set. - let lastMutatingInstructionId = 0; - for (const id of mutatingIdentifiers) { - if (id.mutableRange.end > lastMutatingInstructionId) { - lastMutatingInstructionId = id.mutableRange.end; - } - } - - /* - * Update mutableRange.end for all aliases in this set ending before the - * last mutation. - */ - for (const alias of aliasSet) { - if ( - alias.mutableRange.end < lastMutatingInstructionId && - !isRefOrRefValue(alias) - ) { - alias.mutableRange.end = lastMutatingInstructionId as InstructionId; - } - } - } - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 871c8296c35c..d051fb37250a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -19,6 +19,7 @@ import { DeclarationId, Environment, FunctionExpression, + GeneratedSource, HIRFunction, Hole, IdentifierId, @@ -34,6 +35,7 @@ import { Phi, Place, SpreadPattern, + Type, ValueReason, } from '../HIR'; import { @@ -43,12 +45,6 @@ import { eachTerminalSuccessor, } from '../HIR/visitors'; import {Ok, Result} from '../Utils/Result'; -import { - getArgumentEffect, - getFunctionCallSignature, - isKnownMutableEffect, - mergeValueKinds, -} from './InferReferenceEffects'; import { assertExhaustive, getOrInsertDefault, @@ -65,7 +61,6 @@ import { printSourceLocation, } from '../HIR/PrintHIR'; import {FunctionSignature} from '../HIR/ObjectShape'; -import {getWriteErrorReason} from './InferFunctionEffects'; import prettyFormat from 'pretty-format'; import {createTemporaryPlace} from '../HIR/HIRBuilder'; import { @@ -450,7 +445,6 @@ function applySignature( const reason = getWriteErrorReason({ kind: value.kind, reason: value.reason, - context: new Set(), }); const variable = effect.value.identifier.name !== null && @@ -1074,7 +1068,6 @@ function applyEffect( const reason = getWriteErrorReason({ kind: value.kind, reason: value.reason, - context: new Set(), }); const variable = effect.value.identifier.name !== null && @@ -2567,3 +2560,196 @@ export type AbstractValue = { kind: ValueKind; reason: ReadonlySet; }; + +export function getWriteErrorReason(abstractValue: AbstractValue): string { + if (abstractValue.reason.has(ValueReason.Global)) { + return 'Modifying a variable defined outside a component or hook is not allowed. Consider using an effect'; + } else if (abstractValue.reason.has(ValueReason.JsxCaptured)) { + return 'Modifying a value used previously in JSX is not allowed. Consider moving the modification before the JSX'; + } else if (abstractValue.reason.has(ValueReason.Context)) { + return `Modifying a value returned from 'useContext()' is not allowed.`; + } else if (abstractValue.reason.has(ValueReason.KnownReturnSignature)) { + return 'Modifying a value returned from a function whose return value should not be mutated'; + } else if (abstractValue.reason.has(ValueReason.ReactiveFunctionArgument)) { + return 'Modifying component props or hook arguments is not allowed. Consider using a local variable instead'; + } else if (abstractValue.reason.has(ValueReason.State)) { + return "Modifying a value returned from 'useState()', which should not be modified directly. Use the setter function to update instead"; + } else if (abstractValue.reason.has(ValueReason.ReducerState)) { + return "Modifying a value returned from 'useReducer()', which should not be modified directly. Use the dispatch function to update instead"; + } else if (abstractValue.reason.has(ValueReason.Effect)) { + return 'Modifying a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the modification before calling useEffect()'; + } else if (abstractValue.reason.has(ValueReason.HookCaptured)) { + return 'Modifying a value previously passed as an argument to a hook is not allowed. Consider moving the modification before calling the hook'; + } else if (abstractValue.reason.has(ValueReason.HookReturn)) { + return 'Modifying a value returned from a hook is not allowed. Consider moving the modification into the hook where the value is constructed'; + } else { + return 'This modifies a variable that React considers immutable'; + } +} + +function getArgumentEffect( + signatureEffect: Effect | null, + arg: Place | SpreadPattern, +): Effect { + if (signatureEffect != null) { + if (arg.kind === 'Identifier') { + return signatureEffect; + } else if ( + signatureEffect === Effect.Mutate || + signatureEffect === Effect.ConditionallyMutate + ) { + return signatureEffect; + } else { + // see call-spread-argument-mutable-iterator test fixture + if (signatureEffect === Effect.Freeze) { + CompilerError.throwTodo({ + reason: 'Support spread syntax for hook arguments', + loc: arg.place.loc, + }); + } + // effects[i] is Effect.Capture | Effect.Read | Effect.Store + return Effect.ConditionallyMutateIterator; + } + } else { + return Effect.ConditionallyMutate; + } +} + +export function getFunctionCallSignature( + env: Environment, + type: Type, +): FunctionSignature | null { + if (type.kind !== 'Function') { + return null; + } + return env.getFunctionSignature(type); +} + +export function isKnownMutableEffect(effect: Effect): boolean { + switch (effect) { + case Effect.Store: + case Effect.ConditionallyMutate: + case Effect.ConditionallyMutateIterator: + case Effect.Mutate: { + return true; + } + + case Effect.Unknown: { + CompilerError.invariant(false, { + reason: 'Unexpected unknown effect', + description: null, + loc: GeneratedSource, + suggestions: null, + }); + } + case Effect.Read: + case Effect.Capture: + case Effect.Freeze: { + return false; + } + default: { + assertExhaustive(effect, `Unexpected effect \`${effect}\``); + } + } +} + +/** + * Joins two values using the following rules: + * == Effect Transitions == + * + * Freezing an immutable value has not effect: + * ┌───────────────┐ + * │ │ + * ▼ │ Freeze + * ┌──────────────────────────┐ │ + * │ Immutable │──┘ + * └──────────────────────────┘ + * + * Freezing a mutable or maybe-frozen value makes it frozen. Freezing a frozen + * value has no effect: + * ┌───────────────┐ + * ┌─────────────────────────┐ Freeze │ │ + * │ MaybeFrozen │────┐ ▼ │ Freeze + * └─────────────────────────┘ │ ┌──────────────────────────┐ │ + * ├────▶│ Frozen │──┘ + * │ └──────────────────────────┘ + * ┌─────────────────────────┐ │ + * │ Mutable │────┘ + * └─────────────────────────┘ + * + * == Join Lattice == + * - immutable | mutable => mutable + * The justification is that immutable and mutable values are different types, + * and functions can introspect them to tell the difference (if the argument + * is null return early, else if its an object mutate it). + * - frozen | mutable => maybe-frozen + * Frozen values are indistinguishable from mutable values at runtime, so callers + * cannot dynamically avoid mutation of "frozen" values. If a value could be + * frozen we have to distinguish it from a mutable value. But it also isn't known + * frozen yet, so we distinguish as maybe-frozen. + * - immutable | frozen => frozen + * This is subtle and falls out of the above rules. If a value could be any of + * immutable, mutable, or frozen, then at runtime it could either be a primitive + * or a reference type, and callers can't distinguish frozen or not for reference + * types. To ensure that any sequence of joins btw those three states yields the + * correct maybe-frozen, these two have to produce a frozen value. + * - | maybe-frozen => maybe-frozen + * - immutable | context => context + * - mutable | context => context + * - frozen | context => maybe-frozen + * + * ┌──────────────────────────┐ + * │ Immutable │───┐ + * └──────────────────────────┘ │ + * │ ┌─────────────────────────┐ + * ├───▶│ Frozen │──┐ + * ┌──────────────────────────┐ │ └─────────────────────────┘ │ + * │ Frozen │───┤ │ ┌─────────────────────────┐ + * └──────────────────────────┘ │ ├─▶│ MaybeFrozen │ + * │ ┌─────────────────────────┐ │ └─────────────────────────┘ + * ├───▶│ MaybeFrozen │──┘ + * ┌──────────────────────────┐ │ └─────────────────────────┘ + * │ Mutable │───┘ + * └──────────────────────────┘ + */ +function mergeValueKinds(a: ValueKind, b: ValueKind): ValueKind { + if (a === b) { + return a; + } else if (a === ValueKind.MaybeFrozen || b === ValueKind.MaybeFrozen) { + return ValueKind.MaybeFrozen; + // after this a and b differ and neither are MaybeFrozen + } else if (a === ValueKind.Mutable || b === ValueKind.Mutable) { + if (a === ValueKind.Frozen || b === ValueKind.Frozen) { + // frozen | mutable + return ValueKind.MaybeFrozen; + } else if (a === ValueKind.Context || b === ValueKind.Context) { + // context | mutable + return ValueKind.Context; + } else { + // mutable | immutable + return ValueKind.Mutable; + } + } else if (a === ValueKind.Context || b === ValueKind.Context) { + if (a === ValueKind.Frozen || b === ValueKind.Frozen) { + // frozen | context + return ValueKind.MaybeFrozen; + } else { + // context | immutable + return ValueKind.Context; + } + } else if (a === ValueKind.Frozen || b === ValueKind.Frozen) { + return ValueKind.Frozen; + } else if (a === ValueKind.Global || b === ValueKind.Global) { + return ValueKind.Global; + } else { + CompilerError.invariant( + a === ValueKind.Primitive && b == ValueKind.Primitive, + { + reason: `Unexpected value kind in mergeValues()`, + description: `Found kinds ${a} and ${b}`, + loc: GeneratedSource, + }, + ); + return ValueKind.Primitive; + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts deleted file mode 100644 index 1b0856791a18..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ /dev/null @@ -1,2125 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError'; -import {Environment} from '../HIR'; -import { - AbstractValue, - BasicBlock, - BlockId, - CallExpression, - NewExpression, - Effect, - FunctionEffect, - GeneratedSource, - HIRFunction, - IdentifierId, - InstructionKind, - InstructionValue, - MethodCall, - Phi, - Place, - SpreadPattern, - TInstruction, - Type, - ValueKind, - ValueReason, - isArrayType, - isMapType, - isMutableEffect, - isObjectType, - isSetType, -} from '../HIR/HIR'; -import {FunctionSignature} from '../HIR/ObjectShape'; -import { - printIdentifier, - printMixedHIR, - printPlace, - printSourceLocation, -} from '../HIR/PrintHIR'; -import { - eachInstructionOperand, - eachInstructionValueOperand, - eachPatternOperand, - eachTerminalOperand, - eachTerminalSuccessor, -} from '../HIR/visitors'; -import {assertExhaustive, Set_isSuperset} from '../Utils/utils'; -import { - inferTerminalFunctionEffects, - inferInstructionFunctionEffects, - transformFunctionEffectErrors, -} from './InferFunctionEffects'; - -const UndefinedValue: InstructionValue = { - kind: 'Primitive', - loc: GeneratedSource, - value: undefined, -}; - -/* - * For every usage of a value in the given function, infers the effect or action - * taken at that reference. Each reference is inferred as exactly one of: - * - freeze: this usage freezes the value, ie converts it to frozen. This is only inferred - * when the value *may* not already be frozen. - * - frozen: the value is known to already be "owned" by React and is therefore already - * frozen (permanently and transitively immutable). - * - immutable: the value is not owned by React, but is known to be an immutable value - * that therefore cannot ever change. - * - readonly: the value is not frozen or immutable, but this usage of the value does - * not modify it. the value may be mutated by a subsequent reference. Examples include - * referencing the operands of a binary expression, or referencing the items/properties - * of an array or object literal. - * - mutable: the value is not frozen or immutable, and this usage *may* modify it. - * Examples include passing a value to as a function argument or assigning into an object. - * - * Note that the inference follows variable assignment, so assigning a frozen value - * to a different value will infer usages of the other variable as frozen as well. - * - * The inference assumes that the code follows the rules of React: - * - React function arguments are frozen (component props, hook arguments). - * - Hook arguments are frozen at the point the hook is invoked. - * - React function return values are frozen at the point of being returned, - * thus the return value of a hook call is frozen. - * - JSX represents invocation of a React function (the component) and - * therefore all values passed to JSX become frozen at the point the JSX - * is created. - * - * Internally, the inference tracks the approximate type of value held by each variable, - * and iterates over the control flow graph. The inferred effect of reach reference is - * a combination of the operation performed (ie, assignment into an object mutably uses the - * object; an if condition reads the condition) and the type of the value. The types of values - * are: - * - frozen: can be any type so long as the value is known to be owned by React, permanently - * and transitively immutable - * - maybe-frozen: the value may or may not be frozen, conditionally depending on control flow. - * - immutable: a type with value semantics: primitives, records/tuples when standardized. - * - mutable: a type with reference semantics eg array, object, class instance, etc. - * - * When control flow paths converge the types of values are merged together, with the value - * types forming a lattice to ensure convergence. - */ -export default function inferReferenceEffects( - fn: HIRFunction, - options: {isFunctionExpression: boolean} = {isFunctionExpression: false}, -): Array { - /* - * Initial state contains function params - * TODO: include module declarations here as well - */ - const initialState = InferenceState.empty( - fn.env, - options.isFunctionExpression, - ); - const value: InstructionValue = { - kind: 'Primitive', - loc: fn.loc, - value: undefined, - }; - initialState.initialize(value, { - kind: ValueKind.Frozen, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - - for (const ref of fn.context) { - // TODO(gsn): This is a hack. - const value: InstructionValue = { - kind: 'ObjectExpression', - properties: [], - loc: ref.loc, - }; - initialState.initialize(value, { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set([ref]), - }); - initialState.define(ref, value); - } - - const paramKind: AbstractValue = options.isFunctionExpression - ? { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - } - : { - kind: ValueKind.Frozen, - reason: new Set([ValueReason.ReactiveFunctionArgument]), - context: new Set(), - }; - - if (fn.fnType === 'Component') { - CompilerError.invariant(fn.params.length <= 2, { - reason: - 'Expected React component to have not more than two parameters: one for props and for ref', - description: null, - loc: fn.loc, - suggestions: null, - }); - const [props, ref] = fn.params; - let value: InstructionValue; - let place: Place; - if (props) { - inferParam(props, initialState, paramKind); - } - if (ref) { - if (ref.kind === 'Identifier') { - place = ref; - value = { - kind: 'ObjectExpression', - properties: [], - loc: ref.loc, - }; - } else { - place = ref.place; - value = { - kind: 'ObjectExpression', - properties: [], - loc: ref.place.loc, - }; - } - initialState.initialize(value, { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - initialState.define(place, value); - } - } else { - for (const param of fn.params) { - inferParam(param, initialState, paramKind); - } - } - - // Map of blocks to the last (merged) incoming state that was processed - const statesByBlock: Map = new Map(); - - /* - * Multiple predecessors may be visited prior to reaching a given successor, - * so track the list of incoming state for each successor block. - * These are merged when reaching that block again. - */ - const queuedStates: Map = new Map(); - function queue(blockId: BlockId, state: InferenceState): void { - let queuedState = queuedStates.get(blockId); - if (queuedState != null) { - // merge the queued states for this block - state = queuedState.merge(state) ?? queuedState; - queuedStates.set(blockId, state); - } else { - /* - * this is the first queued state for this block, see whether - * there are changed relative to the last time it was processed. - */ - const prevState = statesByBlock.get(blockId); - const nextState = prevState != null ? prevState.merge(state) : state; - if (nextState != null) { - queuedStates.set(blockId, nextState); - } - } - } - queue(fn.body.entry, initialState); - - const functionEffects: Array = fn.effects ?? []; - - while (queuedStates.size !== 0) { - for (const [blockId, block] of fn.body.blocks) { - const incomingState = queuedStates.get(blockId); - queuedStates.delete(blockId); - if (incomingState == null) { - continue; - } - - statesByBlock.set(blockId, incomingState); - const state = incomingState.clone(); - inferBlock(fn.env, state, block, functionEffects); - - for (const nextBlockId of eachTerminalSuccessor(block.terminal)) { - queue(nextBlockId, state); - } - } - } - - if (options.isFunctionExpression) { - fn.effects = functionEffects; - return []; - } else { - return transformFunctionEffectErrors(functionEffects); - } -} - -type FreezeAction = {values: Set; reason: Set}; - -// Maintains a mapping of top-level variables to the kind of value they hold -class InferenceState { - env: Environment; - #isFunctionExpression: boolean; - - // The kind of each value, based on its allocation site - #values: Map; - /* - * The set of values pointed to by each identifier. This is a set - * to accomodate phi points (where a variable may have different - * values from different control flow paths). - */ - #variables: Map>; - - constructor( - env: Environment, - isFunctionExpression: boolean, - values: Map, - variables: Map>, - ) { - this.env = env; - this.#isFunctionExpression = isFunctionExpression; - this.#values = values; - this.#variables = variables; - } - - static empty( - env: Environment, - isFunctionExpression: boolean, - ): InferenceState { - return new InferenceState(env, isFunctionExpression, new Map(), new Map()); - } - - get isFunctionExpression(): boolean { - return this.#isFunctionExpression; - } - - // (Re)initializes a @param value with its default @param kind. - initialize(value: InstructionValue, kind: AbstractValue): void { - CompilerError.invariant(value.kind !== 'LoadLocal', { - reason: - 'Expected all top-level identifiers to be defined as variables, not values', - description: null, - loc: value.loc, - suggestions: null, - }); - this.#values.set(value, kind); - } - - values(place: Place): Array { - const values = this.#variables.get(place.identifier.id); - CompilerError.invariant(values != null, { - reason: `[hoisting] Expected value kind to be initialized`, - description: `${printPlace(place)}`, - loc: place.loc, - suggestions: null, - }); - return Array.from(values); - } - - // Lookup the kind of the given @param value. - kind(place: Place): AbstractValue { - const values = this.#variables.get(place.identifier.id); - CompilerError.invariant(values != null, { - reason: `[hoisting] Expected value kind to be initialized`, - description: `${printPlace(place)}`, - loc: place.loc, - suggestions: null, - }); - let mergedKind: AbstractValue | null = null; - for (const value of values) { - const kind = this.#values.get(value)!; - mergedKind = - mergedKind !== null ? mergeAbstractValues(mergedKind, kind) : kind; - } - CompilerError.invariant(mergedKind !== null, { - reason: `InferReferenceEffects::kind: Expected at least one value`, - description: `No value found at \`${printPlace(place)}\``, - loc: place.loc, - suggestions: null, - }); - return mergedKind; - } - - // Updates the value at @param place to point to the same value as @param value. - alias(place: Place, value: Place): void { - const values = this.#variables.get(value.identifier.id); - CompilerError.invariant(values != null, { - reason: `[hoisting] Expected value for identifier to be initialized`, - description: `${printIdentifier(value.identifier)}`, - loc: value.loc, - suggestions: null, - }); - this.#variables.set(place.identifier.id, new Set(values)); - } - - // Defines (initializing or updating) a variable with a specific kind of value. - define(place: Place, value: InstructionValue): void { - CompilerError.invariant(this.#values.has(value), { - reason: `Expected value to be initialized at '${printSourceLocation( - value.loc, - )}'`, - description: null, - loc: value.loc, - suggestions: null, - }); - this.#variables.set(place.identifier.id, new Set([value])); - } - - isDefined(place: Place): boolean { - return this.#variables.has(place.identifier.id); - } - - /* - * Records that a given Place was accessed with the given kind and: - * - Updates the effect of @param place based on the kind of value - * and the kind of reference (@param effectKind). - * - Updates the value kind to reflect the effect of the reference. - * - * Notably, a mutable reference is downgraded to readonly if the - * value unless the value is known to be mutable. - * - * Similarly, a freeze reference is converted to readonly if the - * value is already frozen or is immutable. - */ - referenceAndRecordEffects( - freezeActions: Array, - place: Place, - effectKind: Effect, - reason: ValueReason, - ): void { - const values = this.#variables.get(place.identifier.id); - if (values === undefined) { - CompilerError.invariant(effectKind !== Effect.Store, { - reason: '[InferReferenceEffects] Unhandled store reference effect', - description: null, - loc: place.loc, - suggestions: null, - }); - place.effect = - effectKind === Effect.ConditionallyMutate - ? Effect.ConditionallyMutate - : Effect.Read; - return; - } - - const action = this.reference(place, effectKind, reason); - action && freezeActions.push(action); - } - - freezeValues(values: Set, reason: Set): void { - for (const value of values) { - if ( - value.kind === 'DeclareContext' || - (value.kind === 'StoreContext' && - (value.lvalue.kind === InstructionKind.Let || - value.lvalue.kind === InstructionKind.Const)) - ) { - /** - * Avoid freezing context variable declarations, hoisted or otherwise - * function Component() { - * const cb = useBar(() => foo(2)); // produces a hoisted context declaration - * const foo = useFoo(); // reassigns to the context variable - * return ; - * } - */ - continue; - } - this.#values.set(value, { - kind: ValueKind.Frozen, - reason, - context: new Set(), - }); - if ( - value.kind === 'FunctionExpression' && - (this.env.config.enablePreserveExistingMemoizationGuarantees || - this.env.config.enableTransitivelyFreezeFunctionExpressions) - ) { - for (const operand of value.loweredFunc.func.context) { - const operandValues = this.#variables.get(operand.identifier.id); - if (operandValues !== undefined) { - this.freezeValues(operandValues, reason); - } - } - } - } - } - - reference( - place: Place, - effectKind: Effect, - reason: ValueReason, - ): null | FreezeAction { - const values = this.#variables.get(place.identifier.id); - CompilerError.invariant(values !== undefined, { - reason: '[InferReferenceEffects] Expected value to be initialized', - description: null, - loc: place.loc, - suggestions: null, - }); - let valueKind: AbstractValue | null = this.kind(place); - let effect: Effect | null = null; - let freeze: null | FreezeAction = null; - switch (effectKind) { - case Effect.Freeze: { - if ( - valueKind.kind === ValueKind.Mutable || - valueKind.kind === ValueKind.Context || - valueKind.kind === ValueKind.MaybeFrozen - ) { - const reasonSet = new Set([reason]); - effect = Effect.Freeze; - valueKind = { - kind: ValueKind.Frozen, - reason: reasonSet, - context: new Set(), - }; - freeze = {values, reason: reasonSet}; - } else { - effect = Effect.Read; - } - break; - } - case Effect.ConditionallyMutate: { - if ( - valueKind.kind === ValueKind.Mutable || - valueKind.kind === ValueKind.Context - ) { - effect = Effect.ConditionallyMutate; - } else { - effect = Effect.Read; - } - break; - } - case Effect.ConditionallyMutateIterator: { - if ( - valueKind.kind === ValueKind.Mutable || - valueKind.kind === ValueKind.Context - ) { - if ( - isArrayType(place.identifier) || - isSetType(place.identifier) || - isMapType(place.identifier) - ) { - effect = Effect.Capture; - } else { - effect = Effect.ConditionallyMutate; - } - } else { - effect = Effect.Read; - } - break; - } - case Effect.Mutate: { - effect = Effect.Mutate; - break; - } - case Effect.Store: { - /* - * TODO(gsn): This should be bailout once we add bailout infra. - * - * invariant( - * valueKind.kind === ValueKindKind.Mutable, - * `expected valueKind to be 'Mutable' but found to be \`${valueKind}\`` - * ); - */ - effect = isObjectType(place.identifier) ? Effect.Store : Effect.Mutate; - break; - } - case Effect.Capture: { - if ( - valueKind.kind === ValueKind.Primitive || - valueKind.kind === ValueKind.Global || - valueKind.kind === ValueKind.Frozen || - valueKind.kind === ValueKind.MaybeFrozen - ) { - effect = Effect.Read; - } else { - effect = Effect.Capture; - } - break; - } - case Effect.Read: { - effect = Effect.Read; - break; - } - case Effect.Unknown: { - CompilerError.invariant(false, { - reason: - 'Unexpected unknown effect, expected to infer a precise effect kind', - description: null, - loc: place.loc, - suggestions: null, - }); - } - default: { - assertExhaustive( - effectKind, - `Unexpected reference kind \`${effectKind as any as string}\``, - ); - } - } - CompilerError.invariant(effect !== null, { - reason: 'Expected effect to be set', - description: null, - loc: place.loc, - suggestions: null, - }); - place.effect = effect; - return freeze; - } - - /* - * Combine the contents of @param this and @param other, returning a new - * instance with the combined changes _if_ there are any changes, or - * returning null if no changes would occur. Changes include: - * - new entries in @param other that did not exist in @param this - * - entries whose values differ in @param this and @param other, - * and where joining the values produces a different value than - * what was in @param this. - * - * Note that values are joined using a lattice operation to ensure - * termination. - */ - merge(other: InferenceState): InferenceState | null { - let nextValues: Map | null = null; - let nextVariables: Map> | null = null; - - for (const [id, thisValue] of this.#values) { - const otherValue = other.#values.get(id); - if (otherValue !== undefined) { - const mergedValue = mergeAbstractValues(thisValue, otherValue); - if (mergedValue !== thisValue) { - nextValues = nextValues ?? new Map(this.#values); - nextValues.set(id, mergedValue); - } - } - } - for (const [id, otherValue] of other.#values) { - if (this.#values.has(id)) { - // merged above - continue; - } - nextValues = nextValues ?? new Map(this.#values); - nextValues.set(id, otherValue); - } - - for (const [id, thisValues] of this.#variables) { - const otherValues = other.#variables.get(id); - if (otherValues !== undefined) { - let mergedValues: Set | null = null; - for (const otherValue of otherValues) { - if (!thisValues.has(otherValue)) { - mergedValues = mergedValues ?? new Set(thisValues); - mergedValues.add(otherValue); - } - } - if (mergedValues !== null) { - nextVariables = nextVariables ?? new Map(this.#variables); - nextVariables.set(id, mergedValues); - } - } - } - for (const [id, otherValues] of other.#variables) { - if (this.#variables.has(id)) { - continue; - } - nextVariables = nextVariables ?? new Map(this.#variables); - nextVariables.set(id, new Set(otherValues)); - } - - if (nextVariables === null && nextValues === null) { - return null; - } else { - return new InferenceState( - this.env, - this.#isFunctionExpression, - nextValues ?? new Map(this.#values), - nextVariables ?? new Map(this.#variables), - ); - } - } - - /* - * Returns a copy of this state. - * TODO: consider using persistent data structures to make - * clone cheaper. - */ - clone(): InferenceState { - return new InferenceState( - this.env, - this.#isFunctionExpression, - new Map(this.#values), - new Map(this.#variables), - ); - } - - /* - * For debugging purposes, dumps the state to a plain - * object so that it can printed as JSON. - */ - debug(): any { - const result: any = {values: {}, variables: {}}; - const objects: Map = new Map(); - function identify(value: InstructionValue): number { - let id = objects.get(value); - if (id == null) { - id = objects.size; - objects.set(value, id); - } - return id; - } - for (const [value, kind] of this.#values) { - const id = identify(value); - result.values[id] = {kind, value: printMixedHIR(value)}; - } - for (const [variable, values] of this.#variables) { - result.variables[`$${variable}`] = [...values].map(identify); - } - return result; - } - - inferPhi(phi: Phi): void { - const values: Set = new Set(); - for (const [_, operand] of phi.operands) { - const operandValues = this.#variables.get(operand.identifier.id); - // This is a backedge that will be handled later by State.merge - if (operandValues === undefined) continue; - for (const v of operandValues) { - values.add(v); - } - } - - if (values.size > 0) { - this.#variables.set(phi.place.identifier.id, values); - } - } -} - -function inferParam( - param: Place | SpreadPattern, - initialState: InferenceState, - paramKind: AbstractValue, -): void { - let value: InstructionValue; - let place: Place; - if (param.kind === 'Identifier') { - place = param; - value = { - kind: 'Primitive', - loc: param.loc, - value: undefined, - }; - } else { - place = param.place; - value = { - kind: 'Primitive', - loc: param.place.loc, - value: undefined, - }; - } - initialState.initialize(value, paramKind); - initialState.define(place, value); -} - -/* - * Joins two values using the following rules: - * == Effect Transitions == - * - * Freezing an immutable value has not effect: - * ┌───────────────┐ - * │ │ - * ▼ │ Freeze - * ┌──────────────────────────┐ │ - * │ Immutable │──┘ - * └──────────────────────────┘ - * - * Freezing a mutable or maybe-frozen value makes it frozen. Freezing a frozen - * value has no effect: - * ┌───────────────┐ - * ┌─────────────────────────┐ Freeze │ │ - * │ MaybeFrozen │────┐ ▼ │ Freeze - * └─────────────────────────┘ │ ┌──────────────────────────┐ │ - * ├────▶│ Frozen │──┘ - * │ └──────────────────────────┘ - * ┌─────────────────────────┐ │ - * │ Mutable │────┘ - * └─────────────────────────┘ - * - * == Join Lattice == - * - immutable | mutable => mutable - * The justification is that immutable and mutable values are different types, - * and functions can introspect them to tell the difference (if the argument - * is null return early, else if its an object mutate it). - * - frozen | mutable => maybe-frozen - * Frozen values are indistinguishable from mutable values at runtime, so callers - * cannot dynamically avoid mutation of "frozen" values. If a value could be - * frozen we have to distinguish it from a mutable value. But it also isn't known - * frozen yet, so we distinguish as maybe-frozen. - * - immutable | frozen => frozen - * This is subtle and falls out of the above rules. If a value could be any of - * immutable, mutable, or frozen, then at runtime it could either be a primitive - * or a reference type, and callers can't distinguish frozen or not for reference - * types. To ensure that any sequence of joins btw those three states yields the - * correct maybe-frozen, these two have to produce a frozen value. - * - | maybe-frozen => maybe-frozen - * - immutable | context => context - * - mutable | context => context - * - frozen | context => maybe-frozen - * - * ┌──────────────────────────┐ - * │ Immutable │───┐ - * └──────────────────────────┘ │ - * │ ┌─────────────────────────┐ - * ├───▶│ Frozen │──┐ - * ┌──────────────────────────┐ │ └─────────────────────────┘ │ - * │ Frozen │───┤ │ ┌─────────────────────────┐ - * └──────────────────────────┘ │ ├─▶│ MaybeFrozen │ - * │ ┌─────────────────────────┐ │ └─────────────────────────┘ - * ├───▶│ MaybeFrozen │──┘ - * ┌──────────────────────────┐ │ └─────────────────────────┘ - * │ Mutable │───┘ - * └──────────────────────────┘ - */ -export function mergeValueKinds(a: ValueKind, b: ValueKind): ValueKind { - if (a === b) { - return a; - } else if (a === ValueKind.MaybeFrozen || b === ValueKind.MaybeFrozen) { - return ValueKind.MaybeFrozen; - // after this a and b differ and neither are MaybeFrozen - } else if (a === ValueKind.Mutable || b === ValueKind.Mutable) { - if (a === ValueKind.Frozen || b === ValueKind.Frozen) { - // frozen | mutable - return ValueKind.MaybeFrozen; - } else if (a === ValueKind.Context || b === ValueKind.Context) { - // context | mutable - return ValueKind.Context; - } else { - // mutable | immutable - return ValueKind.Mutable; - } - } else if (a === ValueKind.Context || b === ValueKind.Context) { - if (a === ValueKind.Frozen || b === ValueKind.Frozen) { - // frozen | context - return ValueKind.MaybeFrozen; - } else { - // context | immutable - return ValueKind.Context; - } - } else if (a === ValueKind.Frozen || b === ValueKind.Frozen) { - return ValueKind.Frozen; - } else if (a === ValueKind.Global || b === ValueKind.Global) { - return ValueKind.Global; - } else { - CompilerError.invariant( - a === ValueKind.Primitive && b == ValueKind.Primitive, - { - reason: `Unexpected value kind in mergeValues()`, - description: `Found kinds ${a} and ${b}`, - loc: GeneratedSource, - }, - ); - return ValueKind.Primitive; - } -} - -function mergeAbstractValues( - a: AbstractValue, - b: AbstractValue, -): AbstractValue { - const kind = mergeValueKinds(a.kind, b.kind); - if ( - kind === a.kind && - kind === b.kind && - Set_isSuperset(a.reason, b.reason) && - Set_isSuperset(a.context, b.context) - ) { - return a; - } - const reason = new Set(a.reason); - for (const r of b.reason) { - reason.add(r); - } - const context = new Set(a.context); - for (const c of b.context) { - context.add(c); - } - return {kind, reason, context}; -} - -type Continuation = - | { - kind: 'initialize'; - valueKind: AbstractValue; - effect: {kind: Effect; reason: ValueReason} | null; - lvalueEffect?: Effect; - } - | {kind: 'funeffects'}; - -/* - * Iterates over the given @param block, defining variables and - * recording references on the @param state according to JS semantics. - */ -function inferBlock( - env: Environment, - state: InferenceState, - block: BasicBlock, - functionEffects: Array, -): void { - for (const phi of block.phis) { - state.inferPhi(phi); - } - - for (const instr of block.instructions) { - const instrValue = instr.value; - const defaultLvalueEffect = Effect.ConditionallyMutate; - let continuation: Continuation; - const freezeActions: Array = []; - switch (instrValue.kind) { - case 'BinaryExpression': { - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: { - kind: Effect.Read, - reason: ValueReason.Other, - }, - }; - break; - } - case 'ArrayExpression': { - const contextRefOperands = getContextRefOperand(state, instrValue); - const valueKind: AbstractValue = - contextRefOperands.length > 0 - ? { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set(contextRefOperands), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; - - for (const element of instrValue.elements) { - if (element.kind === 'Spread') { - state.referenceAndRecordEffects( - freezeActions, - element.place, - Effect.ConditionallyMutateIterator, - ValueReason.Other, - ); - } else if (element.kind === 'Identifier') { - state.referenceAndRecordEffects( - freezeActions, - element, - Effect.Capture, - ValueReason.Other, - ); - } else { - let _: 'Hole' = element.kind; - } - } - state.initialize(instrValue, valueKind); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.Store; - continuation = { - kind: 'funeffects', - }; - break; - } - case 'NewExpression': { - inferCallEffects( - state, - instr as TInstruction, - freezeActions, - getFunctionCallSignature(env, instrValue.callee.identifier.type), - ); - continuation = {kind: 'funeffects'}; - break; - } - case 'ObjectExpression': { - const contextRefOperands = getContextRefOperand(state, instrValue); - const valueKind: AbstractValue = - contextRefOperands.length > 0 - ? { - kind: ValueKind.Context, - reason: new Set([ValueReason.Other]), - context: new Set(contextRefOperands), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; - - for (const property of instrValue.properties) { - switch (property.kind) { - case 'ObjectProperty': { - if (property.key.kind === 'computed') { - // Object keys must be primitives, so we know they're frozen at this point - state.referenceAndRecordEffects( - freezeActions, - property.key.name, - Effect.Freeze, - ValueReason.Other, - ); - } - // Object construction captures but does not modify the key/property values - state.referenceAndRecordEffects( - freezeActions, - property.place, - Effect.Capture, - ValueReason.Other, - ); - break; - } - case 'Spread': { - // Object construction captures but does not modify the key/property values - state.referenceAndRecordEffects( - freezeActions, - property.place, - Effect.Capture, - ValueReason.Other, - ); - break; - } - default: { - assertExhaustive( - property, - `Unexpected property kind \`${(property as any).kind}\``, - ); - } - } - } - - state.initialize(instrValue, valueKind); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'UnaryExpression': { - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: {kind: Effect.Read, reason: ValueReason.Other}, - }; - break; - } - case 'UnsupportedNode': { - // TODO: handle other statement kinds - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: null, - }; - break; - } - case 'JsxExpression': { - if (instrValue.tag.kind === 'Identifier') { - state.referenceAndRecordEffects( - freezeActions, - instrValue.tag, - Effect.Freeze, - ValueReason.JsxCaptured, - ); - } - if (instrValue.children !== null) { - for (const child of instrValue.children) { - state.referenceAndRecordEffects( - freezeActions, - child, - Effect.Freeze, - ValueReason.JsxCaptured, - ); - } - } - for (const attr of instrValue.props) { - if (attr.kind === 'JsxSpreadAttribute') { - state.referenceAndRecordEffects( - freezeActions, - attr.argument, - Effect.Freeze, - ValueReason.JsxCaptured, - ); - } else { - state.referenceAndRecordEffects( - freezeActions, - attr.place, - Effect.Freeze, - ValueReason.JsxCaptured, - ); - } - } - - state.initialize(instrValue, { - kind: ValueKind.Frozen, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.ConditionallyMutate; - continuation = {kind: 'funeffects'}; - break; - } - case 'JsxFragment': { - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Frozen, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: { - kind: Effect.Freeze, - reason: ValueReason.Other, - }, - }; - break; - } - case 'TemplateLiteral': { - /* - * template literal (with no tag function) always produces - * an immutable string - */ - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: {kind: Effect.Read, reason: ValueReason.Other}, - }; - break; - } - case 'RegExpLiteral': { - // RegExp instances are mutable objects - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: { - kind: Effect.ConditionallyMutate, - reason: ValueReason.Other, - }, - }; - break; - } - case 'MetaProperty': { - if (instrValue.meta !== 'import' || instrValue.property !== 'meta') { - continuation = {kind: 'funeffects'}; - break; - } - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Global, - reason: new Set([ValueReason.Global]), - context: new Set(), - }, - effect: null, - }; - break; - } - case 'LoadGlobal': - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Global, - reason: new Set([ValueReason.Global]), - context: new Set(), - }, - effect: null, - }; - break; - case 'Debugger': - case 'JSXText': - case 'Primitive': { - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: null, - }; - break; - } - case 'ObjectMethod': - case 'FunctionExpression': { - let hasMutableOperand = false; - for (const operand of eachInstructionOperand(instr)) { - CompilerError.invariant(operand.effect !== Effect.Unknown, { - reason: 'Expected fn effects to be populated', - loc: operand.loc, - }); - state.referenceAndRecordEffects( - freezeActions, - operand, - operand.effect, - ValueReason.Other, - ); - hasMutableOperand ||= isMutableEffect(operand.effect, operand.loc); - } - /* - * If a closure did not capture any mutable values, then we can consider it to be - * frozen, which allows it to be independently memoized. - */ - state.initialize(instrValue, { - kind: hasMutableOperand ? ValueKind.Mutable : ValueKind.Frozen, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'TaggedTemplateExpression': { - const operands = [...eachInstructionValueOperand(instrValue)]; - if (operands.length !== 1) { - // future-proofing to make sure we update this case when we support interpolation - CompilerError.throwTodo({ - reason: 'Support tagged template expressions with interpolations', - loc: instrValue.loc, - }); - } - const signature = getFunctionCallSignature( - env, - instrValue.tag.identifier.type, - ); - let calleeEffect = - signature?.calleeEffect ?? Effect.ConditionallyMutate; - const returnValueKind: AbstractValue = - signature !== null - ? { - kind: signature.returnValueKind, - reason: new Set([ - signature.returnValueReason ?? - ValueReason.KnownReturnSignature, - ]), - context: new Set(), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; - state.referenceAndRecordEffects( - freezeActions, - instrValue.tag, - calleeEffect, - ValueReason.Other, - ); - state.initialize(instrValue, returnValueKind); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.ConditionallyMutate; - continuation = {kind: 'funeffects'}; - break; - } - case 'CallExpression': { - inferCallEffects( - state, - instr as TInstruction, - freezeActions, - getFunctionCallSignature(env, instrValue.callee.identifier.type), - ); - continuation = {kind: 'funeffects'}; - break; - } - case 'MethodCall': { - CompilerError.invariant(state.isDefined(instrValue.receiver), { - reason: - '[InferReferenceEffects] Internal error: receiver of PropertyCall should have been defined by corresponding PropertyLoad', - description: null, - loc: instrValue.loc, - suggestions: null, - }); - state.referenceAndRecordEffects( - freezeActions, - instrValue.property, - Effect.Read, - ValueReason.Other, - ); - inferCallEffects( - state, - instr as TInstruction, - freezeActions, - getFunctionCallSignature(env, instrValue.property.identifier.type), - ); - continuation = {kind: 'funeffects'}; - break; - } - case 'PropertyStore': { - const effect = - state.kind(instrValue.object).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - effect, - ValueReason.Other, - ); - state.referenceAndRecordEffects( - freezeActions, - instrValue.object, - Effect.Store, - ValueReason.Other, - ); - - const lvalue = instr.lvalue; - state.alias(lvalue, instrValue.value); - lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'PropertyDelete': { - // `delete` returns a boolean (immutable) and modifies the object - continuation = { - kind: 'initialize', - valueKind: { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - effect: {kind: Effect.Mutate, reason: ValueReason.Other}, - }; - break; - } - case 'PropertyLoad': { - state.referenceAndRecordEffects( - freezeActions, - instrValue.object, - Effect.Read, - ValueReason.Other, - ); - const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; - state.initialize(instrValue, state.kind(instrValue.object)); - state.define(lvalue, instrValue); - continuation = {kind: 'funeffects'}; - break; - } - case 'ComputedStore': { - const effect = - state.kind(instrValue.object).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - effect, - ValueReason.Other, - ); - state.referenceAndRecordEffects( - freezeActions, - instrValue.property, - Effect.Capture, - ValueReason.Other, - ); - state.referenceAndRecordEffects( - freezeActions, - instrValue.object, - Effect.Store, - ValueReason.Other, - ); - - const lvalue = instr.lvalue; - state.alias(lvalue, instrValue.value); - lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'ComputedDelete': { - state.referenceAndRecordEffects( - freezeActions, - instrValue.object, - Effect.Mutate, - ValueReason.Other, - ); - state.referenceAndRecordEffects( - freezeActions, - instrValue.property, - Effect.Read, - ValueReason.Other, - ); - state.initialize(instrValue, { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.Mutate; - continuation = {kind: 'funeffects'}; - break; - } - case 'ComputedLoad': { - state.referenceAndRecordEffects( - freezeActions, - instrValue.object, - Effect.Read, - ValueReason.Other, - ); - state.referenceAndRecordEffects( - freezeActions, - instrValue.property, - Effect.Read, - ValueReason.Other, - ); - const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; - state.initialize(instrValue, state.kind(instrValue.object)); - state.define(lvalue, instrValue); - continuation = {kind: 'funeffects'}; - break; - } - case 'Await': { - state.initialize(instrValue, state.kind(instrValue.value)); - /* - * Awaiting a value causes it to change state (go from unresolved to resolved or error) - * It also means that any side-effects which would occur as part of the promise evaluation - * will occur. - */ - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - Effect.ConditionallyMutate, - ValueReason.Other, - ); - const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; - state.alias(lvalue, instrValue.value); - continuation = {kind: 'funeffects'}; - break; - } - case 'TypeCastExpression': { - /* - * A type cast expression has no effect at runtime, so it's equivalent to a raw - * identifier: - * ``` - * x = (y: type) // is equivalent to... - * x = y - * ``` - */ - state.initialize(instrValue, state.kind(instrValue.value)); - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - Effect.Read, - ValueReason.Other, - ); - const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; - state.alias(lvalue, instrValue.value); - continuation = {kind: 'funeffects'}; - break; - } - case 'StartMemoize': - case 'FinishMemoize': { - for (const val of eachInstructionValueOperand(instrValue)) { - if (env.config.enablePreserveExistingMemoizationGuarantees) { - state.referenceAndRecordEffects( - freezeActions, - val, - Effect.Freeze, - ValueReason.Other, - ); - } else { - state.referenceAndRecordEffects( - freezeActions, - val, - Effect.Read, - ValueReason.Other, - ); - } - } - const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; - state.initialize(instrValue, { - kind: ValueKind.Frozen, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - state.define(lvalue, instrValue); - continuation = {kind: 'funeffects'}; - break; - } - case 'LoadLocal': { - /** - * Due to backedges in the CFG, we may revisit LoadLocal lvalues - * multiple times. Unlike StoreLocal which may reassign to existing - * identifiers, LoadLocal always evaluates to store a new temporary. - * This means that we should always model LoadLocal as a Capture effect - * on the rvalue. - */ - const lvalue = instr.lvalue; - state.referenceAndRecordEffects( - freezeActions, - instrValue.place, - Effect.Capture, - ValueReason.Other, - ); - lvalue.effect = Effect.ConditionallyMutate; - // direct aliasing: `a = b`; - state.alias(lvalue, instrValue.place); - continuation = {kind: 'funeffects'}; - break; - } - case 'LoadContext': { - state.referenceAndRecordEffects( - freezeActions, - instrValue.place, - Effect.Capture, - ValueReason.Other, - ); - const lvalue = instr.lvalue; - lvalue.effect = Effect.ConditionallyMutate; - const valueKind = state.kind(instrValue.place); - state.initialize(instrValue, valueKind); - state.define(lvalue, instrValue); - continuation = {kind: 'funeffects'}; - break; - } - case 'DeclareLocal': { - const value = UndefinedValue; - state.initialize( - value, - // Catch params may be aliased to mutable values - instrValue.lvalue.kind === InstructionKind.Catch - ? { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - } - : { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - ); - state.define(instrValue.lvalue.place, value); - continuation = {kind: 'funeffects'}; - break; - } - case 'DeclareContext': { - state.initialize(instrValue, { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - state.define(instrValue.lvalue.place, instrValue); - continuation = {kind: 'funeffects'}; - break; - } - case 'PostfixUpdate': - case 'PrefixUpdate': { - const effect = - state.isDefined(instrValue.lvalue) && - state.kind(instrValue.lvalue).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - effect, - ValueReason.Other, - ); - - const lvalue = instr.lvalue; - state.alias(lvalue, instrValue.value); - lvalue.effect = Effect.Store; - state.alias(instrValue.lvalue, instrValue.value); - /* - * NOTE: *not* using state.reference since this is an assignment. - * reference() checks if the effect is valid given the value kind, - * but here the previous value kind doesn't matter since we are - * replacing it - */ - instrValue.lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'StoreLocal': { - const effect = - state.isDefined(instrValue.lvalue.place) && - state.kind(instrValue.lvalue.place).kind === ValueKind.Context - ? Effect.ConditionallyMutate - : Effect.Capture; - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - effect, - ValueReason.Other, - ); - - const lvalue = instr.lvalue; - state.alias(lvalue, instrValue.value); - lvalue.effect = Effect.Store; - state.alias(instrValue.lvalue.place, instrValue.value); - /* - * NOTE: *not* using state.reference since this is an assignment. - * reference() checks if the effect is valid given the value kind, - * but here the previous value kind doesn't matter since we are - * replacing it - */ - instrValue.lvalue.place.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'StoreContext': { - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - Effect.ConditionallyMutate, - ValueReason.Other, - ); - state.referenceAndRecordEffects( - freezeActions, - instrValue.lvalue.place, - Effect.Mutate, - ValueReason.Other, - ); - - const lvalue = instr.lvalue; - if (instrValue.lvalue.kind !== InstructionKind.Reassign) { - state.initialize(instrValue, { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }); - state.define(instrValue.lvalue.place, instrValue); - } - state.alias(lvalue, instrValue.value); - lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'StoreGlobal': { - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - Effect.Capture, - ValueReason.Other, - ); - const lvalue = instr.lvalue; - lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'Destructure': { - let effect: Effect = Effect.Capture; - for (const place of eachPatternOperand(instrValue.lvalue.pattern)) { - if ( - state.isDefined(place) && - state.kind(place).kind === ValueKind.Context - ) { - effect = Effect.ConditionallyMutate; - break; - } - } - state.referenceAndRecordEffects( - freezeActions, - instrValue.value, - effect, - ValueReason.Other, - ); - - const lvalue = instr.lvalue; - state.alias(lvalue, instrValue.value); - lvalue.effect = Effect.Store; - for (const place of eachPatternOperand(instrValue.lvalue.pattern)) { - state.alias(place, instrValue.value); - /* - * NOTE: *not* using state.reference since this is an assignment. - * reference() checks if the effect is valid given the value kind, - * but here the previous value kind doesn't matter since we are - * replacing it - */ - place.effect = Effect.Store; - } - continuation = {kind: 'funeffects'}; - break; - } - case 'GetIterator': { - /** - * This instruction represents the step of retrieving an iterator from the collection - * in `for (... of )` syntax. We model two cases: - * - * 1. The collection is immutable or a known collection type (e.g. Array). In this case - * we infer that the iterator produced won't be the same as the collection itself. - * If the collection is an Array, this is because it will produce a native Array - * iterator. If the collection is already frozen, we assume it must be of some - * type that returns a separate iterator. In theory you could pass an Iterator - * as props to a component and then for..of over that in the component body, but - * this already violates React's rules so we assume you're not doing this. - * 2. The collection could be an Iterator itself, such that advancing the iterator - * (modeled with IteratorNext) mutates the collection itself. - */ - const kind = state.kind(instrValue.collection).kind; - const isMutable = - kind === ValueKind.Mutable || kind === ValueKind.Context; - let effect; - let valueKind: AbstractValue; - const iterator = instrValue.collection.identifier; - if ( - !isMutable || - isArrayType(iterator) || - isMapType(iterator) || - isSetType(iterator) - ) { - // Case 1, assume iterator is a separate mutable object - effect = { - kind: Effect.Read, - reason: ValueReason.Other, - }; - valueKind = { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; - } else { - // Case 2, assume that the iterator could be the (mutable) collection itself - effect = { - kind: Effect.Capture, - reason: ValueReason.Other, - }; - valueKind = state.kind(instrValue.collection); - } - continuation = { - kind: 'initialize', - effect, - valueKind, - lvalueEffect: Effect.Store, - }; - break; - } - case 'IteratorNext': { - /** - * This instruction represents advancing an iterator with .next(). We use a - * conditional mutate to model the two cases for GetIterator: - * - If the collection is a mutable iterator, we want to model the fact that - * advancing the iterator will mutate it - * - If the iterator may be different from the collection and the collection - * is frozen, we don't want to report a false positive "cannot mutate" error. - * - * ConditionallyMutate reflects this "mutate if mutable" semantic. - */ - state.referenceAndRecordEffects( - freezeActions, - instrValue.iterator, - Effect.ConditionallyMutateIterator, - ValueReason.Other, - ); - /** - * Regardless of the effect on the iterator, the *result* of advancing the iterator - * is to extract a value from the collection. We use a Capture effect to reflect this - * aliasing, and then initialize() the lvalue to the same kind as the colleciton to - * ensure that the item is mutable or frozen if the collection is mutable/frozen. - */ - state.referenceAndRecordEffects( - freezeActions, - instrValue.collection, - Effect.Capture, - ValueReason.Other, - ); - state.initialize(instrValue, state.kind(instrValue.collection)); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = Effect.Store; - continuation = {kind: 'funeffects'}; - break; - } - case 'NextPropertyOf': { - continuation = { - kind: 'initialize', - effect: {kind: Effect.Read, reason: ValueReason.Other}, - lvalueEffect: Effect.Store, - valueKind: { - kind: ValueKind.Primitive, - reason: new Set([ValueReason.Other]), - context: new Set(), - }, - }; - break; - } - default: { - assertExhaustive(instrValue, 'Unexpected instruction kind'); - } - } - - if (continuation.kind === 'initialize') { - for (const operand of eachInstructionOperand(instr)) { - CompilerError.invariant(continuation.effect != null, { - reason: `effectKind must be set for instruction value \`${instrValue.kind}\``, - description: null, - loc: instrValue.loc, - suggestions: null, - }); - state.referenceAndRecordEffects( - freezeActions, - operand, - continuation.effect.kind, - continuation.effect.reason, - ); - } - - state.initialize(instrValue, continuation.valueKind); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = continuation.lvalueEffect ?? defaultLvalueEffect; - } - - functionEffects.push(...inferInstructionFunctionEffects(env, state, instr)); - freezeActions.forEach(({values, reason}) => - state.freezeValues(values, reason), - ); - } - - const terminalFreezeActions: Array = []; - for (const operand of eachTerminalOperand(block.terminal)) { - let effect; - if (block.terminal.kind === 'return' || block.terminal.kind === 'throw') { - if ( - state.isDefined(operand) && - ((operand.identifier.type.kind === 'Function' && - state.isFunctionExpression) || - state.kind(operand).kind === ValueKind.Context) - ) { - /** - * Returned values should only be typed as 'frozen' if they are both (1) - * local and (2) not a function expression which may capture and mutate - * this function's outer context. - */ - effect = Effect.ConditionallyMutate; - } else { - effect = Effect.Freeze; - } - } else { - effect = Effect.Read; - } - state.referenceAndRecordEffects( - terminalFreezeActions, - operand, - effect, - ValueReason.Other, - ); - } - functionEffects.push(...inferTerminalFunctionEffects(state, block)); - terminalFreezeActions.forEach(({values, reason}) => - state.freezeValues(values, reason), - ); -} - -function getContextRefOperand( - state: InferenceState, - instrValue: InstructionValue, -): Array { - const result = []; - for (const place of eachInstructionValueOperand(instrValue)) { - if ( - state.isDefined(place) && - state.kind(place).kind === ValueKind.Context - ) { - result.push(place); - } - } - return result; -} - -export function getFunctionCallSignature( - env: Environment, - type: Type, -): FunctionSignature | null { - if (type.kind !== 'Function') { - return null; - } - return env.getFunctionSignature(type); -} - -/* - * Make a best attempt at matching arguments of a {@link MethodCall} to parameter effects. - * defined in its {@link FunctionSignature}. - * - * @param fn - * @param sig - * @returns Inferred effects of function arguments, or null if inference fails. - */ -export function getFunctionEffects( - fn: MethodCall | CallExpression | NewExpression, - sig: FunctionSignature, -): Array | null { - const results = []; - for (let i = 0; i < fn.args.length; i++) { - const arg = fn.args[i]; - if (i < sig.positionalParams.length) { - /* - * Only infer effects when there is a direct mapping positional arg --> positional param - * Otherwise, return null to indicate inference failed - */ - if (arg.kind === 'Identifier') { - results.push(sig.positionalParams[i]); - } else { - return null; - } - } else if (sig.restParam !== null) { - results.push(sig.restParam); - } else { - /* - * If there are more arguments than positional arguments and a rest parameter is not - * defined, we'll also assume that inference failed - */ - return null; - } - } - return results; -} - -export function isKnownMutableEffect(effect: Effect): boolean { - switch (effect) { - case Effect.Store: - case Effect.ConditionallyMutate: - case Effect.ConditionallyMutateIterator: - case Effect.Mutate: { - return true; - } - - case Effect.Unknown: { - CompilerError.invariant(false, { - reason: 'Unexpected unknown effect', - description: null, - loc: GeneratedSource, - suggestions: null, - }); - } - case Effect.Read: - case Effect.Capture: - case Effect.Freeze: { - return false; - } - default: { - assertExhaustive(effect, `Unexpected effect \`${effect}\``); - } - } -} -/** - * Returns true if all of the arguments are both non-mutable (immutable or frozen) - * _and_ are not functions which might mutate their arguments. Note that function - * expressions count as frozen so long as they do not mutate free variables: this - * function checks that such functions also don't mutate their inputs. - */ -function areArgumentsImmutableAndNonMutating( - state: InferenceState, - args: MethodCall['args'], -): boolean { - for (const arg of args) { - if (arg.kind === 'Identifier' && arg.identifier.type.kind === 'Function') { - const fnShape = state.env.getFunctionSignature(arg.identifier.type); - if (fnShape != null) { - return ( - !fnShape.positionalParams.some(isKnownMutableEffect) && - (fnShape.restParam == null || - !isKnownMutableEffect(fnShape.restParam)) - ); - } - } - const place = arg.kind === 'Identifier' ? arg : arg.place; - - const kind = state.kind(place).kind; - switch (kind) { - case ValueKind.Primitive: - case ValueKind.Frozen: { - /* - * Only immutable values, or frozen lambdas are allowed. - * A lambda may appear frozen even if it may mutate its inputs, - * so we have a second check even for frozen value types - */ - break; - } - default: { - /** - * Globals, module locals, and other locally defined functions may - * mutate their arguments. - */ - return false; - } - } - const values = state.values(place); - for (const value of values) { - if ( - value.kind === 'FunctionExpression' && - value.loweredFunc.func.params.some(param => { - const place = param.kind === 'Identifier' ? param : param.place; - const range = place.identifier.mutableRange; - return range.end > range.start + 1; - }) - ) { - // This is a function which may mutate its inputs - return false; - } - } - } - return true; -} - -export function getArgumentEffect( - signatureEffect: Effect | null, - arg: Place | SpreadPattern, -): Effect { - if (signatureEffect != null) { - if (arg.kind === 'Identifier') { - return signatureEffect; - } else if ( - signatureEffect === Effect.Mutate || - signatureEffect === Effect.ConditionallyMutate - ) { - return signatureEffect; - } else { - // see call-spread-argument-mutable-iterator test fixture - if (signatureEffect === Effect.Freeze) { - CompilerError.throwTodo({ - reason: 'Support spread syntax for hook arguments', - loc: arg.place.loc, - }); - } - // effects[i] is Effect.Capture | Effect.Read | Effect.Store - return Effect.ConditionallyMutateIterator; - } - } else { - return Effect.ConditionallyMutate; - } -} - -function inferCallEffects( - state: InferenceState, - instr: - | TInstruction - | TInstruction - | TInstruction, - freezeActions: Array, - signature: FunctionSignature | null, -): void { - const instrValue = instr.value; - const returnValueKind: AbstractValue = - signature !== null - ? { - kind: signature.returnValueKind, - reason: new Set([ - signature.returnValueReason ?? ValueReason.KnownReturnSignature, - ]), - context: new Set(), - } - : { - kind: ValueKind.Mutable, - reason: new Set([ValueReason.Other]), - context: new Set(), - }; - - if ( - instrValue.kind === 'MethodCall' && - signature !== null && - signature.mutableOnlyIfOperandsAreMutable && - areArgumentsImmutableAndNonMutating(state, instrValue.args) - ) { - /* - * None of the args are mutable or mutate their params, we can downgrade to - * treating as all reads (except that the receiver may be captured) - */ - for (const arg of instrValue.args) { - const place = arg.kind === 'Identifier' ? arg : arg.place; - state.referenceAndRecordEffects( - freezeActions, - place, - Effect.Read, - ValueReason.Other, - ); - } - state.referenceAndRecordEffects( - freezeActions, - instrValue.receiver, - Effect.Capture, - ValueReason.Other, - ); - state.initialize(instrValue, returnValueKind); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = - instrValue.receiver.effect === Effect.Capture - ? Effect.Store - : Effect.ConditionallyMutate; - return; - } - - const effects = - signature !== null ? getFunctionEffects(instrValue, signature) : null; - let hasCaptureArgument = false; - for (let i = 0; i < instrValue.args.length; i++) { - const arg = instrValue.args[i]; - const place = arg.kind === 'Identifier' ? arg : arg.place; - /* - * If effects are inferred for an argument, we should fail invalid - * mutating effects - */ - state.referenceAndRecordEffects( - freezeActions, - place, - getArgumentEffect(effects != null ? effects[i] : null, arg), - ValueReason.Other, - ); - hasCaptureArgument ||= place.effect === Effect.Capture; - } - const callee = - instrValue.kind === 'MethodCall' ? instrValue.receiver : instrValue.callee; - if (signature !== null) { - state.referenceAndRecordEffects( - freezeActions, - callee, - signature.calleeEffect, - ValueReason.Other, - ); - } else { - /** - * For new expressions, we infer a `read` effect on the Class / Function type - * to avoid extending mutable ranges of locally created classes, e.g. - * ```js - * const MyClass = getClass(); - * const value = new MyClass(val1, val2) - * ^ (read) ^ (conditionally mutate) - * ``` - * - * Risks: - * Classes / functions created during render could technically capture and - * mutate their enclosing scope, which we currently do not detect. - */ - - state.referenceAndRecordEffects( - freezeActions, - callee, - instrValue.kind === 'NewExpression' - ? Effect.Read - : Effect.ConditionallyMutate, - ValueReason.Other, - ); - } - hasCaptureArgument ||= callee.effect === Effect.Capture; - - state.initialize(instrValue, returnValueKind); - state.define(instr.lvalue, instrValue); - instr.lvalue.effect = hasCaptureArgument - ? Effect.Store - : Effect.ConditionallyMutate; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferTryCatchAliases.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferTryCatchAliases.ts deleted file mode 100644 index 3b33160820c6..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferTryCatchAliases.ts +++ /dev/null @@ -1,49 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {BlockId, HIRFunction, Identifier} from '../HIR'; -import DisjointSet from '../Utils/DisjointSet'; - -/* - * Any values created within a try/catch block could be aliased to the try handler. - * Our lowering ensures that every instruction within a try block will be lowered into a - * basic block ending in a maybe-throw terminal that points to its catch block, so we can - * iterate such blocks and alias their instruction lvalues to the handler's param (if present). - */ -export function inferTryCatchAliases( - fn: HIRFunction, - aliases: DisjointSet, -): void { - const handlerParams: Map = new Map(); - for (const [_, block] of fn.body.blocks) { - if ( - block.terminal.kind === 'try' && - block.terminal.handlerBinding !== null - ) { - handlerParams.set( - block.terminal.handler, - block.terminal.handlerBinding.identifier, - ); - } else if (block.terminal.kind === 'maybe-throw') { - const handlerParam = handlerParams.get(block.terminal.handler); - if (handlerParam === undefined) { - /* - * There's no catch clause param, nothing to alias to so - * skip this block - */ - continue; - } - /* - * Otherwise alias all values created in this block to the - * catch clause param - */ - for (const instr of block.instructions) { - aliases.union([handlerParam, instr.lvalue.identifier]); - } - } - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts index 93b99fb38526..eb645cc218dc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/index.ts @@ -7,8 +7,6 @@ export {default as analyseFunctions} from './AnalyseFunctions'; export {dropManualMemoization} from './DropManualMemoization'; -export {inferMutableRanges} from './InferMutableRanges'; export {inferReactivePlaces} from './InferReactivePlaces'; -export {default as inferReferenceEffects} from './InferReferenceEffects'; export {inlineImmediatelyInvokedFunctionExpressions} from './InlineImmediatelyInvokedFunctionExpressions'; export {inferEffectDependencies} from './InferEffectDependencies'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts index 8484a1ca8d93..3afb00b71a81 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -24,7 +24,6 @@ import { getHookKind, isMutableEffect, } from '../HIR'; -import {getFunctionCallSignature} from '../Inference/InferReferenceEffects'; import {assertExhaustive, getOrInsertDefault} from '../Utils/utils'; import {getPlaceScope, ReactiveScope} from '../HIR/HIR'; import { @@ -35,6 +34,7 @@ import { visitReactiveFunction, } from './visitors'; import {printPlace} from '../HIR/PrintHIR'; +import {getFunctionCallSignature} from '../Inference/InferMutationAliasingEffects'; /* * This pass prunes reactive scopes that are not necessary to bound downstream computation. diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts index 31bbf8c94d35..163d645957c0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts @@ -12,7 +12,7 @@ import { eachInstructionValueOperand, eachTerminalOperand, } from '../HIR/visitors'; -import {getFunctionCallSignature} from '../Inference/InferReferenceEffects'; +import {getFunctionCallSignature} from '../Inference/InferMutationAliasingEffects'; /** * Validates that local variables cannot be reassigned after render. diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts index 7a79c74780e2..f71bb40545bb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts @@ -125,10 +125,7 @@ export function validateNoFreezingKnownMutableFunctions( ); if (knownMutation && knownMutation.kind === 'ContextMutation') { contextMutationEffects.set(lvalue.identifier.id, knownMutation); - } else if ( - fn.env.config.enableNewMutationAliasingModel && - value.loweredFunc.func.aliasingEffects != null - ) { + } else if (value.loweredFunc.func.aliasingEffects != null) { const context = new Set( value.loweredFunc.func.context.map(p => p.identifier.id), ); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts index 85adb79ceb85..e7f8b1c0e5b1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoImpureFunctionsInRender.ts @@ -7,7 +7,7 @@ import {CompilerDiagnostic, CompilerError, ErrorSeverity} from '..'; import {HIRFunction} from '../HIR'; -import {getFunctionCallSignature} from '../Inference/InferReferenceEffects'; +import {getFunctionCallSignature} from '../Inference/InferMutationAliasingEffects'; import {Result} from '../Utils/Result'; /** diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.expect.md new file mode 100644 index 000000000000..90ffebd69583 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel:false +function Component() { + const foo = () => { + someGlobal = true; + }; + // spreading a function is weird, but it doesn't call the function so this is allowed + return
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel:false +function Component() { + const $ = _c(1); + const foo = _temp; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 =
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} +function _temp() { + someGlobal = true; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.js similarity index 61% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.js index e749f10f78fb..2ed0c70a7f96 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-assigning-to-global-in-function-spread-as-jsx.js @@ -3,5 +3,6 @@ function Component() { const foo = () => { someGlobal = true; }; + // spreading a function is weird, but it doesn't call the function so this is allowed return
; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md deleted file mode 100644 index 7d14f2a5dc8e..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md +++ /dev/null @@ -1,107 +0,0 @@ - -## Input - -```javascript -// @flow @enableTransitivelyFreezeFunctionExpressions:false @enableNewMutationAliasingModel:false -import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * 1. `InferMutableRanges` derives the mutable range of identifiers and their - * aliases from `LoadLocal`, `PropertyLoad`, etc - * - After this pass, y's mutable range only extends to `arrayPush(x, y)` - * - We avoid assigning mutable ranges to loads after y's mutable range, as - * these are working with an immutable value. As a result, `LoadLocal y` and - * `PropertyLoad y` do not get mutable ranges - * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes, - * as according to the 'co-mutation' of different values - * - Here, we infer that - * - `arrayPush(y, x)` might alias `x` and `y` to each other - * - `setPropertyKey(x, ...)` may mutate both `x` and `y` - * - This pass correctly extends the mutable range of `y` - * - Since we didn't run `InferMutableRange` logic again, the LoadLocal / - * PropertyLoads still don't have a mutable range - * - * Note that the this bug is an edge case. Compiler output is only invalid for: - * - function expressions with - * `enableTransitivelyFreezeFunctionExpressions:false` - * - functions that throw and get retried without clearing the memocache - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- */ -function useFoo({a, b}: {a: number, b: number}) { - const x = []; - const y = {value: a}; - - arrayPush(x, y); // x and y co-mutate - const y_alias = y; - const cb = () => y_alias.value; - setPropertyByKey(x[0], 'value', b); // might overwrite y.value - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2, b: 10}], - sequentialRenders: [ - {a: 2, b: 10}, - {a: 2, b: 11}, - ], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime"; - -function useFoo(t0) { - const $ = _c(5); - const { a, b } = t0; - let t1; - if ($[0] !== a || $[1] !== b) { - const x = []; - const y = { value: a }; - - arrayPush(x, y); - const y_alias = y; - let t2; - if ($[3] !== y_alias.value) { - t2 = () => y_alias.value; - $[3] = y_alias.value; - $[4] = t2; - } else { - t2 = $[4]; - } - const cb = t2; - setPropertyByKey(x[0], "value", b); - t1 = ; - $[0] = a; - $[1] = b; - $[2] = t1; - } else { - t1 = $[2]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{ a: 2, b: 10 }], - sequentialRenders: [ - { a: 2, b: 10 }, - { a: 2, b: 11 }, - ], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js deleted file mode 100644 index 911c06e64466..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js +++ /dev/null @@ -1,53 +0,0 @@ -// @flow @enableTransitivelyFreezeFunctionExpressions:false @enableNewMutationAliasingModel:false -import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * 1. `InferMutableRanges` derives the mutable range of identifiers and their - * aliases from `LoadLocal`, `PropertyLoad`, etc - * - After this pass, y's mutable range only extends to `arrayPush(x, y)` - * - We avoid assigning mutable ranges to loads after y's mutable range, as - * these are working with an immutable value. As a result, `LoadLocal y` and - * `PropertyLoad y` do not get mutable ranges - * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes, - * as according to the 'co-mutation' of different values - * - Here, we infer that - * - `arrayPush(y, x)` might alias `x` and `y` to each other - * - `setPropertyKey(x, ...)` may mutate both `x` and `y` - * - This pass correctly extends the mutable range of `y` - * - Since we didn't run `InferMutableRange` logic again, the LoadLocal / - * PropertyLoads still don't have a mutable range - * - * Note that the this bug is an edge case. Compiler output is only invalid for: - * - function expressions with - * `enableTransitivelyFreezeFunctionExpressions:false` - * - functions that throw and get retried without clearing the memocache - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- */ -function useFoo({a, b}: {a: number, b: number}) { - const x = []; - const y = {value: a}; - - arrayPush(x, y); // x and y co-mutate - const y_alias = y; - const cb = () => y_alias.value; - setPropertyByKey(x[0], 'value', b); // might overwrite y.value - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2, b: 10}], - sequentialRenders: [ - {a: 2, b: 10}, - {a: 2, b: 11}, - ], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md deleted file mode 100644 index 698562dad18d..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md +++ /dev/null @@ -1,87 +0,0 @@ - -## Input - -```javascript -// @flow @enableTransitivelyFreezeFunctionExpressions:false @enableNewMutationAliasingModel:false -import {setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * Variation of bug in `bug-aliased-capture-aliased-mutate` - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- */ - -function useFoo({a}: {a: number, b: number}) { - const arr = []; - const obj = {value: a}; - - setPropertyByKey(obj, 'arr', arr); - const obj_alias = obj; - const cb = () => obj_alias.arr.length; - for (let i = 0; i < a; i++) { - arr.push(i); - } - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2}], - sequentialRenders: [{a: 2}, {a: 3}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { setPropertyByKey, Stringify } from "shared-runtime"; - -function useFoo(t0) { - const $ = _c(4); - const { a } = t0; - let t1; - if ($[0] !== a) { - const arr = []; - const obj = { value: a }; - - setPropertyByKey(obj, "arr", arr); - const obj_alias = obj; - let t2; - if ($[2] !== obj_alias.arr.length) { - t2 = () => obj_alias.arr.length; - $[2] = obj_alias.arr.length; - $[3] = t2; - } else { - t2 = $[3]; - } - const cb = t2; - for (let i = 0; i < a; i++) { - arr.push(i); - } - - t1 = ; - $[0] = a; - $[1] = t1; - } else { - t1 = $[1]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{ a: 2 }], - sequentialRenders: [{ a: 2 }, { a: 3 }], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js deleted file mode 100644 index 1311a9dcfa69..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js +++ /dev/null @@ -1,34 +0,0 @@ -// @flow @enableTransitivelyFreezeFunctionExpressions:false @enableNewMutationAliasingModel:false -import {setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * Variation of bug in `bug-aliased-capture-aliased-mutate` - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- */ - -function useFoo({a}: {a: number, b: number}) { - const arr = []; - const obj = {value: a}; - - setPropertyByKey(obj, 'arr', arr); - const obj_alias = obj; - const cb = () => obj_alias.arr.length; - for (let i = 0; i < a; i++) { - arr.push(i); - } - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2}], - sequentialRenders: [{a: 2}, {a: 3}], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md index ea33e361e3ba..8d1e85222558 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md @@ -85,19 +85,11 @@ import { makeArray, mutate } from "shared-runtime"; * used when we analyze CallExpressions. */ function Component(t0) { - const $ = _c(5); + const $ = _c(3); const { foo, bar } = t0; - let t1; - if ($[0] !== foo) { - t1 = { foo }; - $[0] = foo; - $[1] = t1; - } else { - t1 = $[1]; - } - const x = t1; let y; - if ($[2] !== bar || $[3] !== x) { + if ($[0] !== bar || $[1] !== foo) { + const x = { foo }; y = { bar }; const f0 = function () { const a = makeArray(y); @@ -108,11 +100,11 @@ function Component(t0) { f0(); mutate(y.x); - $[2] = bar; - $[3] = x; - $[4] = y; + $[0] = bar; + $[1] = foo; + $[2] = y; } else { - y = $[4]; + y = $[2]; } return y; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md deleted file mode 100644 index 9c874fa68ebc..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.expect.md +++ /dev/null @@ -1,92 +0,0 @@ - -## Input - -```javascript -// @enableNewMutationAliasingModel:false -import {CONST_TRUE, Stringify, mutate, useIdentity} from 'shared-runtime'; - -/** - * Fixture showing an edge case for ReactiveScope variable propagation. - * - * Found differences in evaluator results - * Non-forget (expected): - *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- * Forget: - *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- * [[ (exception in render) Error: invariant broken ]] - * - */ -function Component() { - const obj = CONST_TRUE ? {inner: {value: 'hello'}} : null; - const boxedInner = [obj?.inner]; - useIdentity(null); - mutate(obj); - if (boxedInner[0] !== obj?.inner) { - throw new Error('invariant broken'); - } - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{arg: 0}], - sequentialRenders: [{arg: 0}, {arg: 1}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel:false -import { CONST_TRUE, Stringify, mutate, useIdentity } from "shared-runtime"; - -/** - * Fixture showing an edge case for ReactiveScope variable propagation. - * - * Found differences in evaluator results - * Non-forget (expected): - *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- * Forget: - *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- * [[ (exception in render) Error: invariant broken ]] - * - */ -function Component() { - const $ = _c(4); - const obj = CONST_TRUE ? { inner: { value: "hello" } } : null; - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = [obj?.inner]; - $[0] = t0; - } else { - t0 = $[0]; - } - const boxedInner = t0; - useIdentity(null); - mutate(obj); - if (boxedInner[0] !== obj?.inner) { - throw new Error("invariant broken"); - } - let t1; - if ($[1] !== boxedInner || $[2] !== obj) { - t1 = ; - $[1] = boxedInner; - $[2] = obj; - $[3] = t1; - } else { - t1 = $[3]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ arg: 0 }], - sequentialRenders: [{ arg: 0 }, { arg: 1 }], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx deleted file mode 100644 index 1a7c996a9e29..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-phi-as-dependency.tsx +++ /dev/null @@ -1,31 +0,0 @@ -// @enableNewMutationAliasingModel:false -import {CONST_TRUE, Stringify, mutate, useIdentity} from 'shared-runtime'; - -/** - * Fixture showing an edge case for ReactiveScope variable propagation. - * - * Found differences in evaluator results - * Non-forget (expected): - *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- * Forget: - *
{"obj":{"inner":{"value":"hello"},"wat0":"joe"},"inner":["[[ cyclic ref *2 ]]"]}
- * [[ (exception in render) Error: invariant broken ]] - * - */ -function Component() { - const obj = CONST_TRUE ? {inner: {value: 'hello'}} : null; - const boxedInner = [obj?.inner]; - useIdentity(null); - mutate(obj); - if (boxedInner[0] !== obj?.inner) { - throw new Error('invariant broken'); - } - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{arg: 0}], - sequentialRenders: [{arg: 0}, {arg: 1}], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md deleted file mode 100644 index 93098b916d72..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md +++ /dev/null @@ -1,110 +0,0 @@ - -## Input - -```javascript -// @enableNewMutationAliasingModel:false -import {identity, mutate} from 'shared-runtime'; - -/** - * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr - * with the mutation hoisted to a named variable instead of being directly - * inlined into the Object key. - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * Forget: - * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] - */ -function Component(props) { - const key = {}; - const tmp = (mutate(key), key); - const context = { - // Here, `tmp` is frozen (as it's inferred to be a primitive/string) - [tmp]: identity([props.value]), - }; - mutate(key); - return [context, key]; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{value: 42}], - sequentialRenders: [{value: 42}, {value: 42}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel:false -import { identity, mutate } from "shared-runtime"; - -/** - * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr - * with the mutation hoisted to a named variable instead of being directly - * inlined into the Object key. - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * Forget: - * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] - */ -function Component(props) { - const $ = _c(8); - let key; - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - key = {}; - t0 = (mutate(key), key); - $[0] = key; - $[1] = t0; - } else { - key = $[0]; - t0 = $[1]; - } - const tmp = t0; - let t1; - if ($[2] !== props.value) { - t1 = identity([props.value]); - $[2] = props.value; - $[3] = t1; - } else { - t1 = $[3]; - } - let t2; - if ($[4] !== t1) { - t2 = { [tmp]: t1 }; - $[4] = t1; - $[5] = t2; - } else { - t2 = $[5]; - } - const context = t2; - - mutate(key); - let t3; - if ($[6] !== context) { - t3 = [context, key]; - $[6] = context; - $[7] = t3; - } else { - t3 = $[7]; - } - return t3; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{ value: 42 }], - sequentialRenders: [{ value: 42 }, { value: 42 }], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js deleted file mode 100644 index 620f5eeb17af..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js +++ /dev/null @@ -1,32 +0,0 @@ -// @enableNewMutationAliasingModel:false -import {identity, mutate} from 'shared-runtime'; - -/** - * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr - * with the mutation hoisted to a named variable instead of being directly - * inlined into the Object key. - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * Forget: - * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] - * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] - */ -function Component(props) { - const key = {}; - const tmp = (mutate(key), key); - const context = { - // Here, `tmp` is frozen (as it's inferred to be a primitive/string) - [tmp]: identity([props.value]), - }; - mutate(key); - return [context, key]; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{value: 42}], - sequentialRenders: [{value: 42}, {value: 42}], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md deleted file mode 100644 index 8476885de773..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md +++ /dev/null @@ -1,33 +0,0 @@ - -## Input - -```javascript -// @enableNewMutationAliasingModel:false -function Component() { - const foo = () => { - someGlobal = true; - }; - return
; -} - -``` - - -## Error - -``` -Found 1 error: - -Error: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) - -error.assign-global-in-jsx-spread-attribute.ts:4:4 - 2 | function Component() { - 3 | const foo = () => { -> 4 | someGlobal = true; - | ^^^^^^^^^^ Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) - 5 | }; - 6 | return
; - 7 | } -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.expect.md deleted file mode 100644 index cc0ad9de117d..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.expect.md +++ /dev/null @@ -1,72 +0,0 @@ - -## Input - -```javascript -// @validateNoFreezingKnownMutableFunctions @enableNewMutationAliasingModel:false - -import {useCallback, useEffect, useRef} from 'react'; -import {useHook} from 'shared-runtime'; - -function Component() { - const params = useHook(); - const update = useCallback( - partialParams => { - const nextParams = { - ...params, - ...partialParams, - }; - nextParams.param = 'value'; - console.log(nextParams); - }, - [params] - ); - const ref = useRef(null); - useEffect(() => { - if (ref.current === null) { - update(); - } - }, [update]); - - return 'ok'; -} - -``` - - -## Error - -``` -Found 1 error: - -Error: Cannot modify local variables after render completes - -This argument is a function which may reassign or mutate a local variable after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead. - -error.bug-old-inference-false-positive-ref-validation-in-use-effect.ts:20:12 - 18 | ); - 19 | const ref = useRef(null); -> 20 | useEffect(() => { - | ^^^^^^^ -> 21 | if (ref.current === null) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 22 | update(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 23 | } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> 24 | }, [update]); - | ^^^^ This function may (indirectly) reassign or modify a local variable after render - 25 | - 26 | return 'ok'; - 27 | } - -error.bug-old-inference-false-positive-ref-validation-in-use-effect.ts:14:6 - 12 | ...partialParams, - 13 | }; -> 14 | nextParams.param = 'value'; - | ^^^^^^^^^^ This modifies a local variable - 15 | console.log(nextParams); - 16 | }, - 17 | [params] -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.js deleted file mode 100644 index b5d70dbd8161..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.bug-old-inference-false-positive-ref-validation-in-use-effect.js +++ /dev/null @@ -1,27 +0,0 @@ -// @validateNoFreezingKnownMutableFunctions @enableNewMutationAliasingModel:false - -import {useCallback, useEffect, useRef} from 'react'; -import {useHook} from 'shared-runtime'; - -function Component() { - const params = useHook(); - const update = useCallback( - partialParams => { - const nextParams = { - ...params, - ...partialParams, - }; - nextParams.param = 'value'; - console.log(nextParams); - }, - [params] - ); - const ref = useRef(null); - useEffect(() => { - if (ref.current === null) { - update(); - } - }, [update]); - - return 'ok'; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.expect.md deleted file mode 100644 index 89bcedf956c7..000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.expect.md +++ /dev/null @@ -1,39 +0,0 @@ - -## Input - -```javascript -// @enableNewMutationAliasingModel:false -function Foo() { - const x = () => { - window.href = 'foo'; - }; - const y = {x}; - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [], -}; - -``` - - -## Error - -``` -Found 1 error: - -Error: Modifying a variable defined outside a component or hook is not allowed. Consider using an effect - -error.object-capture-global-mutation.ts:4:4 - 2 | function Foo() { - 3 | const x = () => { -> 4 | window.href = 'foo'; - | ^^^^^^ Modifying a variable defined outside a component or hook is not allowed. Consider using an effect - 5 | }; - 6 | const y = {x}; - 7 | return ; -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.expect.md new file mode 100644 index 000000000000..9d970ef9e675 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +function Foo() { + const x = () => { + window.href = 'foo'; + }; + const y = {x}; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Foo() { + const $ = _c(1); + const x = _temp; + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const y = { x }; + t0 = ; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} +function _temp() { + window.href = "foo"; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], +}; + +``` + +### Eval output +(kind: exception) Bar is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.js similarity index 81% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.js index d95a0a6265cc..b3c936a2a284 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.object-capture-global-mutation.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/object-expression-captures-function-with-global-mutation.js @@ -1,4 +1,3 @@ -// @enableNewMutationAliasingModel:false function Foo() { const x = () => { window.href = 'foo'; From 5063b3283fcae4bb43756d0d18d32008e3910bea Mon Sep 17 00:00:00 2001 From: Joseph Savona <6425824+josephsavona@users.noreply.github.com> Date: Fri, 15 Aug 2025 15:27:30 -0700 Subject: [PATCH 4/5] [compiler] Remove now-unused FunctionEffect type (#34029) The new mutation/aliasing model significantly expands on the idea of FunctionEffect. The type (and its usage in HIRFunction.effects) was only necessary for the now-deleted old inference model so we can clean up this code now. --- .../src/HIR/HIR.ts | 21 ++--------- .../src/HIR/PrintHIR.ts | 14 +------- .../src/Optimization/LowerContextAccess.ts | 2 +- .../src/Optimization/OutlineJsx.ts | 2 +- ...ValidateNoFreezingKnownMutableFunctions.ts | 36 ++++--------------- 5 files changed, 11 insertions(+), 64 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 51715b3c1e1d..6b3ba6f94c8a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -7,7 +7,7 @@ import {BindingKind} from '@babel/traverse'; import * as t from '@babel/types'; -import {CompilerError, CompilerErrorDetailOptions} from '../CompilerError'; +import {CompilerError} from '../CompilerError'; import {assertExhaustive} from '../Utils/utils'; import {Environment, ReactFunctionType} from './Environment'; import type {HookKind} from './ObjectShape'; @@ -282,30 +282,13 @@ export type HIRFunction = { returnTypeAnnotation: t.FlowType | t.TSType | null; returns: Place; context: Array; - effects: Array | null; body: HIR; generator: boolean; async: boolean; directives: Array; - aliasingEffects?: Array | null; + aliasingEffects: Array | null; }; -export type FunctionEffect = - | { - kind: 'GlobalMutation'; - error: CompilerErrorDetailOptions; - } - | { - kind: 'ReactMutation'; - error: CompilerErrorDetailOptions; - } - | { - kind: 'ContextMutation'; - places: ReadonlySet; - effect: Effect; - loc: SourceLocation; - }; - /* * Each reactive scope may have its own control-flow, so the instructions form * a control-flow graph. The graph comprises a set of basic blocks which reference diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index aa6a7b0c65ce..34ce7f7694b7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -554,23 +554,11 @@ export function printInstructionValue(instrValue: ReactiveValue): string { const context = instrValue.loweredFunc.func.context .map(dep => printPlace(dep)) .join(','); - const effects = - instrValue.loweredFunc.func.effects - ?.map(effect => { - if (effect.kind === 'ContextMutation') { - return `ContextMutation places=[${[...effect.places] - .map(place => printPlace(place)) - .join(', ')}] effect=${effect.effect}`; - } else { - return `GlobalMutation`; - } - }) - .join(', ') ?? ''; const aliasingEffects = instrValue.loweredFunc.func.aliasingEffects ?.map(printAliasingEffect) ?.join(', ') ?? ''; - value = `${kind} ${name} @context[${context}] @effects[${effects}] @aliasingEffects=[${aliasingEffects}]\n${fn}`; + value = `${kind} ${name} @context[${context}] @aliasingEffects=[${aliasingEffects}]\n${fn}`; break; } case 'TaggedTemplateExpression': { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts index 2d0b073a0459..62845934c16f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts @@ -255,7 +255,6 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { returnTypeAnnotation: null, returns: createTemporaryPlace(env, GeneratedSource), context: [], - effects: null, body: { entry: block.id, blocks: new Map([[block.id, block]]), @@ -263,6 +262,7 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { generator: false, async: false, directives: [], + aliasingEffects: [], }; reversePostorderBlocks(fn.body); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts index e59d60271a0d..6232456e620c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/OutlineJsx.ts @@ -370,7 +370,6 @@ function emitOutlinedFn( returnTypeAnnotation: null, returns: createTemporaryPlace(env, GeneratedSource), context: [], - effects: null, body: { entry: block.id, blocks: new Map([[block.id, block]]), @@ -378,6 +377,7 @@ function emitOutlinedFn( generator: false, async: false, directives: [], + aliasingEffects: [], }; return fn; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts index f71bb40545bb..f3543e113a17 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoFreezingKnownMutableFunctions.ts @@ -7,10 +7,8 @@ import {CompilerDiagnostic, CompilerError, Effect, ErrorSeverity} from '..'; import { - FunctionEffect, HIRFunction, IdentifierId, - isMutableEffect, isRefOrRefLikeMutableType, Place, } from '../HIR'; @@ -18,8 +16,8 @@ import { eachInstructionValueOperand, eachTerminalOperand, } from '../HIR/visitors'; +import {AliasingEffect} from '../Inference/AliasingEffects'; import {Result} from '../Utils/Result'; -import {Iterable_some} from '../Utils/utils'; /** * Validates that functions with known mutations (ie due to types) cannot be passed @@ -50,14 +48,14 @@ export function validateNoFreezingKnownMutableFunctions( const errors = new CompilerError(); const contextMutationEffects: Map< IdentifierId, - Extract + Extract > = new Map(); function visitOperand(operand: Place): void { if (operand.effect === Effect.Freeze) { const effect = contextMutationEffects.get(operand.identifier.id); if (effect != null) { - const place = [...effect.places][0]; + const place = effect.value; const variable = place != null && place.identifier.name != null && @@ -77,7 +75,7 @@ export function validateNoFreezingKnownMutableFunctions( }) .withDetail({ kind: 'error', - loc: effect.loc, + loc: effect.value.loc, message: `This modifies ${variable}`, }), ); @@ -108,24 +106,7 @@ export function validateNoFreezingKnownMutableFunctions( break; } case 'FunctionExpression': { - const knownMutation = (value.loweredFunc.func.effects ?? []).find( - effect => { - return ( - effect.kind === 'ContextMutation' && - (effect.effect === Effect.Store || - effect.effect === Effect.Mutate) && - Iterable_some(effect.places, place => { - return ( - isMutableEffect(place.effect, place.loc) && - !isRefOrRefLikeMutableType(place.identifier.type) - ); - }) - ); - }, - ); - if (knownMutation && knownMutation.kind === 'ContextMutation') { - contextMutationEffects.set(lvalue.identifier.id, knownMutation); - } else if (value.loweredFunc.func.aliasingEffects != null) { + if (value.loweredFunc.func.aliasingEffects != null) { const context = new Set( value.loweredFunc.func.context.map(p => p.identifier.id), ); @@ -146,12 +127,7 @@ export function validateNoFreezingKnownMutableFunctions( context.has(effect.value.identifier.id) && !isRefOrRefLikeMutableType(effect.value.identifier.type) ) { - contextMutationEffects.set(lvalue.identifier.id, { - kind: 'ContextMutation', - effect: Effect.Mutate, - loc: effect.value.loc, - places: new Set([effect.value]), - }); + contextMutationEffects.set(lvalue.identifier.id, effect); break effects; } break; From 431bb0bddb640d01d668448f1133e44bd3eb3e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 15 Aug 2025 18:32:27 -0400 Subject: [PATCH 5/5] [DevTools] Mark Unknown Reasons for Suspending with a Note (#34200) We currently only track the reason something might suspend in development mode through debug info but this excludes some cases. As a result we can end up with boundary that suspends but has no cause. This tries to detect that and show a notice for why that might be. I'm also trying to make it work with old React versions to cover everything. In production we don't track any of this meta data like `_debugInfo`, `_debugThenable` etc. so after resolution there's no information to take from. Except suspensey images / css which we can track in prod too. We could track lazy component types already. We'd have to add something that tracks after the fact if something used a lazy child, child as a promise, hooks, etc. which doesn't exist today. So that's not backwards compatible and might add some perf/memory cost. However, another strategy is also to try to replay the components after the fact which could be backwards compatible. That's tricky for child position since there's so many rules for how to do that which would have to be replicated. If you're in development you get a different error. Given that we've added instrumentation very recently. If you're on an older development version of React, then you get a different error. Unfortunately I think my feature test is not quite perfect because it's tricky to test for the instrumentation I just added. https://github.com/facebook/react/pull/34146 So I think for some prereleases that has `_debugOwner` but doesn't have that you'll get a misleading error. Finally, if you're in a modern development environment, the only reason we should have any gaps is because of throw-a-Promise. This will highlight it as missing. We can detect that something threw if a Suspense boundary commits with a RetryCache but since it's a WeakSet we can't look into it to see anything about what it might have been. I don't plan on doing anything to improve this since it would only apply to new versions of React anyway and it's just inherently flawed. So just deprecate it #34032. Note that nothing in here can detect that we suspended Transition. So throwing at the root or in an update won't show that anywhere. --- .../src/backend/fiber/renderer.js | 75 +++++++++++++++++++ .../src/backend/legacy/renderer.js | 2 + .../src/backend/types.js | 2 + .../react-devtools-shared/src/backendAPI.js | 2 + .../react-devtools-shared/src/constants.js | 7 ++ .../InspectedElementSharedStyles.css | 9 +++ .../Components/InspectedElementSuspendedBy.js | 47 +++++++++++- .../src/frontend/types.js | 2 + 8 files changed, 144 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 439d522801a3..94c394cbd701 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -15,6 +15,7 @@ import type { ReactIOInfo, ReactStackTrace, ReactCallSite, + Wakeable, } from 'shared/ReactTypes'; import type {HooksTree} from 'react-debug-tools/src/ReactDebugHooks'; @@ -87,6 +88,10 @@ import { SUSPENSE_TREE_OPERATION_REMOVE, SUSPENSE_TREE_OPERATION_REORDER_CHILDREN, SUSPENSE_TREE_OPERATION_RESIZE, + UNKNOWN_SUSPENDERS_NONE, + UNKNOWN_SUSPENDERS_REASON_PRODUCTION, + UNKNOWN_SUSPENDERS_REASON_OLD_VERSION, + UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE, } from '../../constants'; import {inspectHooksOfFiber} from 'react-debug-tools'; import { @@ -296,6 +301,9 @@ type SuspenseNode = { // Track whether any of the items in suspendedBy are unique this this Suspense boundaries or if they're all // also in the parent sets. This determine whether this could contribute in the loading sequence. hasUniqueSuspenders: boolean, + // Track whether anything suspended in this boundary that we can't track either because it was using throw + // a promise, an older version of React or because we're inspecting prod. + hasUnknownSuspenders: boolean, }; function createSuspenseNode( @@ -309,6 +317,7 @@ function createSuspenseNode( rects: null, suspendedBy: new Map(), hasUniqueSuspenders: false, + hasUnknownSuspenders: false, }); } @@ -2745,6 +2754,8 @@ export function attach( parentSuspenseNode.hasUniqueSuspenders = true; } } + // We have observed at least one known reason this might have been suspended. + parentSuspenseNode.hasUnknownSuspenders = false; // Suspending right below the root is not attributed to any particular component in UI // other than the SuspenseNode and the HostRoot's FiberInstance. const suspendedBy = parentInstance.suspendedBy; @@ -2783,6 +2794,7 @@ export function attach( // It can now be marked as having unique suspenders. We can skip its children // since they'll still be blocked by this one. node.hasUniqueSuspenders = true; + node.hasUnknownSuspenders = false; } else if (node.firstChild !== null) { node = node.firstChild; continue; @@ -3458,6 +3470,25 @@ export function attach( insertSuspendedBy(asyncInfo); } + function trackThrownPromisesFromRetryCache( + suspenseNode: SuspenseNode, + retryCache: ?WeakSet, + ): void { + if (retryCache != null) { + // If a Suspense boundary ever committed in fallback state with a retryCache, that + // suggests that something unique to that boundary was suspensey since otherwise + // it wouldn't have thrown and so never created the retryCache. + // Unfortunately if we don't have any DEV time debug info or debug thenables then + // we have no meta data to show. However, we still mark this Suspense boundary as + // participating in the loading sequence since apparently it can suspend. + suspenseNode.hasUniqueSuspenders = true; + // We have not seen any reason yet for why this suspense node might have been + // suspended but it clearly has been at some point. If we later discover a reason + // we'll clear this flag again. + suspenseNode.hasUnknownSuspenders = true; + } + } + function mountVirtualChildrenRecursively( firstChild: Fiber, lastChild: null | Fiber, // non-inclusive @@ -3749,6 +3780,9 @@ export function attach( } else if (fiber.tag === SuspenseComponent && OffscreenComponent === -1) { // Legacy Suspense without the Offscreen wrapper. For the modern Suspense we just handle the // Offscreen wrapper itself specially. + if (newSuspenseNode !== null) { + trackThrownPromisesFromRetryCache(newSuspenseNode, fiber.stateNode); + } const isTimedOut = fiber.memoizedState !== null; if (isTimedOut) { // Special case: if Suspense mounts in a timed-out state, @@ -3791,6 +3825,9 @@ export function attach( 'There should always be an Offscreen Fiber child in a Suspense boundary.', ); } + + trackThrownPromisesFromRetryCache(newSuspenseNode, fiber.stateNode); + const fallbackFiber = contentFiber.sibling; // First update only the Offscreen boundary. I.e. the main content. @@ -4600,6 +4637,18 @@ export function attach( const prevWasHidden = isOffscreen && prevFiber.memoizedState !== null; const nextIsHidden = isOffscreen && nextFiber.memoizedState !== null; + if (isLegacySuspense) { + if ( + fiberInstance !== null && + fiberInstance.suspenseNode !== null && + (prevFiber.stateNode === null) !== (nextFiber.stateNode === null) + ) { + trackThrownPromisesFromRetryCache( + fiberInstance.suspenseNode, + nextFiber.stateNode, + ); + } + } // The logic below is inspired by the code paths in updateSuspenseComponent() // inside ReactFiberBeginWork in the React source code. if (prevDidTimeout && nextDidTimeOut) { @@ -4726,6 +4775,13 @@ export function attach( const prevFallbackFiber = prevContentFiber.sibling; const nextFallbackFiber = nextContentFiber.sibling; + if ((prevFiber.stateNode === null) !== (nextFiber.stateNode === null)) { + trackThrownPromisesFromRetryCache( + fiberInstance.suspenseNode, + nextFiber.stateNode, + ); + } + // First update only the Offscreen boundary. I.e. the main content. updateFlags |= updateVirtualChildrenRecursively( nextContentFiber, @@ -6100,6 +6156,23 @@ export function attach( getNearestSuspenseNode(fiberInstance), ); + let unknownSuspenders = UNKNOWN_SUSPENDERS_NONE; + if ( + fiberInstance.suspenseNode !== null && + fiberInstance.suspenseNode.hasUnknownSuspenders && + !isTimedOutSuspense + ) { + // Something unknown threw to suspended this boundary. Let's figure out why that might be. + if (renderer.bundleType === 0) { + unknownSuspenders = UNKNOWN_SUSPENDERS_REASON_PRODUCTION; + } else if (!('_debugInfo' in fiber)) { + // TODO: We really should detect _debugThenable and the auto-instrumentation for lazy/thenables too. + unknownSuspenders = UNKNOWN_SUSPENDERS_REASON_OLD_VERSION; + } else { + unknownSuspenders = UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE; + } + } + return { id: fiberInstance.id, @@ -6164,6 +6237,7 @@ export function attach( suspendedBy: suspendedBy, suspendedByRange: suspendedByRange, + unknownSuspenders: unknownSuspenders, // List of owners owners, @@ -6280,6 +6354,7 @@ export function attach( serializeAsyncInfo(info, virtualInstance, null), ), suspendedByRange: suspendedByRange, + unknownSuspenders: UNKNOWN_SUSPENDERS_NONE, // List of owners owners, diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 592238934f5d..d1623ff24bfd 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -34,6 +34,7 @@ import { TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, TREE_OPERATION_REORDER_CHILDREN, + UNKNOWN_SUSPENDERS_NONE, } from '../../constants'; import {decorateMany, forceUpdate, restoreMany} from './utils'; @@ -860,6 +861,7 @@ export function attach( // Not supported in legacy renderers. suspendedBy: [], suspendedByRange: null, + unknownSuspenders: UNKNOWN_SUSPENDERS_NONE, // List of owners owners, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 978b546b8353..ffbacea01aab 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -34,6 +34,7 @@ import type {TimelineDataExport} from 'react-devtools-timeline/src/types'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; import type {ReactFunctionLocation, ReactStackTrace} from 'shared/ReactTypes'; import type Agent from './agent'; +import type {UnknownSuspendersReason} from '../constants'; type BundleType = | 0 // PROD @@ -303,6 +304,7 @@ export type InspectedElement = { // Things that suspended this Instances suspendedBy: Object, // DehydratedData or Array suspendedByRange: null | [number, number], + unknownSuspenders: UnknownSuspendersReason, // List of owners owners: Array | null, diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index eb1b6f6df3ff..f8fa4da37254 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -272,6 +272,7 @@ export function convertInspectedElementBackendToFrontend( warnings, suspendedBy, suspendedByRange, + unknownSuspenders, nativeTag, } = inspectedElementBackend; @@ -317,6 +318,7 @@ export function convertInspectedElementBackendToFrontend( ? [] : hydratedSuspendedBy.map(backendToFrontendSerializedAsyncInfo), suspendedByRange, + unknownSuspenders, nativeTag, }; diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index ce6ed0b308a8..8071d3d4a2c6 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -32,6 +32,13 @@ export const SUSPENSE_TREE_OPERATION_RESIZE = 11; export const PROFILING_FLAG_BASIC_SUPPORT = 0b01; export const PROFILING_FLAG_TIMELINE_SUPPORT = 0b10; +export const UNKNOWN_SUSPENDERS_NONE: UnknownSuspendersReason = 0; // If we had at least one debugInfo, then that might have been the reason. +export const UNKNOWN_SUSPENDERS_REASON_PRODUCTION: UnknownSuspendersReason = 1; // We're running in prod. That might be why we had unknown suspenders. +export const UNKNOWN_SUSPENDERS_REASON_OLD_VERSION: UnknownSuspendersReason = 2; // We're running an old version of React that doesn't have full coverage. That might be the reason. +export const UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE: UnknownSuspendersReason = 3; // If we're in dev, didn't detect and debug info and still suspended (other than CSS/image) the only reason is thrown promise. + +export opaque type UnknownSuspendersReason = 0 | 1 | 2 | 3; + export const LOCAL_STORAGE_DEFAULT_TAB_KEY = 'React::DevTools::defaultTab'; export const LOCAL_STORAGE_COMPONENT_FILTER_PREFERENCES_KEY = 'React::DevTools::componentFilters'; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css index 0fb5107361c1..978077d2d9a2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSharedStyles.css @@ -52,6 +52,15 @@ min-width: 1rem; } +.InfoRow { + border-top: 1px solid var(--color-border); + padding: 0.5rem 1rem; +} + +.InfoRow:last-child { + margin-bottom: -0.25rem; +} + .CollapsableRow { border-top: 1px solid var(--color-border); } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js index 3527c23b0669..451b53b4ac59 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js @@ -27,6 +27,13 @@ import type { } from 'react-devtools-shared/src/frontend/types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; +import { + UNKNOWN_SUSPENDERS_NONE, + UNKNOWN_SUSPENDERS_REASON_PRODUCTION, + UNKNOWN_SUSPENDERS_REASON_OLD_VERSION, + UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE, +} from '../../../constants'; + type RowProps = { bridge: FrontendBridge, element: Element, @@ -295,7 +302,10 @@ export default function InspectedElementSuspendedBy({ const {suspendedBy, suspendedByRange} = inspectedElement; // Skip the section if nothing suspended this component. - if (suspendedBy == null || suspendedBy.length === 0) { + if ( + (suspendedBy == null || suspendedBy.length === 0) && + inspectedElement.unknownSuspenders === UNKNOWN_SUSPENDERS_NONE + ) { return null; } @@ -327,9 +337,41 @@ export default function InspectedElementSuspendedBy({ minTime = maxTime - 25; } - const sortedSuspendedBy = suspendedBy.slice(0); + const sortedSuspendedBy = suspendedBy === null ? [] : suspendedBy.slice(0); sortedSuspendedBy.sort(compareTime); + let unknownSuspenders = null; + switch (inspectedElement.unknownSuspenders) { + case UNKNOWN_SUSPENDERS_REASON_PRODUCTION: + unknownSuspenders = ( +
+ Something suspended but we don't know the exact reason in production + builds of React. Test this in development mode to see exactly what + might suspend. +
+ ); + break; + case UNKNOWN_SUSPENDERS_REASON_OLD_VERSION: + unknownSuspenders = ( +
+ Something suspended but we don't track all the necessary information + in older versions of React. Upgrade to the latest version of React to + see exactly what might suspend. +
+ ); + break; + case UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE: + unknownSuspenders = ( +
+ Something threw a Promise to suspend this boundary. It's likely an + outdated version of a library that doesn't yet fully take advantage of + use(). Upgrade your data fetching library to see exactly what might + suspend. +
+ ); + break; + } + return (
@@ -351,6 +393,7 @@ export default function InspectedElementSuspendedBy({ maxTime={maxTime} /> ))} + {unknownSuspenders}
); } diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index d7d22b9530c9..4aa936ab6481 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -19,6 +19,7 @@ import type { Unserializable, } from 'react-devtools-shared/src/hydration'; import type {ReactFunctionLocation, ReactStackTrace} from 'shared/ReactTypes'; +import type {UnknownSuspendersReason} from '../constants'; export type BrowserTheme = 'dark' | 'light'; @@ -283,6 +284,7 @@ export type InspectedElement = { suspendedBy: Object, // Minimum start time to maximum end time + a potential (not actual) throttle, within the nearest boundary. suspendedByRange: null | [number, number], + unknownSuspenders: UnknownSuspendersReason, // List of owners owners: Array | null,