[compiler][donotcommit] show differences between identifier + scope ranges

[ghstack-poisoned]

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

@@ -98,6 +98,7 @@ import {
 } from "../Validation";
 import { validateLocalsNotReassignedAfterRender } from "../Validation/ValidateLocalsNotReassignedAfterRender";
 import { outlineFunctions } from "../Optimization/OutlineFunctions";
+import { assertMutableRangesAreAlwaysScopes } from "../HIR/BuildReactiveScopeTerminalsHIR";
 
 export type CompilerPipelineValue =
   | { kind: "ast"; name: string; value: CodegenFunction }
@@ -299,6 +300,8 @@ function* runWithEnvironment(
       value: hir,
     });
 
+    assertMutableRangesAreAlwaysScopes(hir);
+
     assertValidBlockNesting(hir);
 
     flattenReactiveLoopsHIR(hir);

compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts

@@ -9,10 +9,74 @@ import {
   GotoVariant,
   HIRFunction,
   InstructionId,
+  MutableRange,
+  Place,
   ReactiveScope,
   ReactiveScopeTerminal,
   ScopeId,
 } from "./HIR";
+import { printPlace } from "./PrintHIR";
+import {
+  eachInstructionLValue,
+  eachInstructionOperand,
+  eachTerminalOperand,
+} from "./visitors";
+
+export function assertMutableRangesAreAlwaysScopes(fn: HIRFunction): void {
+  const scopeRanges: Set<MutableRange> = new Set();
+
+  for (const [_, block] of fn.body.blocks) {
+    if (
+      block.terminal.kind === "scope" ||
+      block.terminal.kind === "pruned-scope"
+    ) {
+      scopeRanges.add(block.terminal.scope.range);
+    }
+  }
+
+  const validate = (place: Place): void => {
+    if (
+      place.identifier.mutableRange.end >
+      place.identifier.mutableRange.start + 1
+    ) {
+      if (scopeRanges.size !== 0) {
+        CompilerError.invariant(
+          scopeRanges.has(place.identifier.mutableRange),
+          {
+            reason: "Mutable range not found in scope ranges!",
+            description: `${printPlace(place)}`,
+            loc: place.loc,
+          }
+        );
+      }
+      if (place.identifier.scope != null) {
+        // Useful for debugging before we build reactive scope terminals
+        CompilerError.invariant(
+          place.identifier.scope.range === place.identifier.mutableRange,
+          {
+            reason: "Mutable range inconsistent with range of attached scope",
+            description: `${printPlace(place)}`,
+            loc: place.loc,
+          }
+        );
+      }
+    }
+  };
+
+  for (const [_, block] of fn.body.blocks) {
+    for (const instr of block.instructions) {
+      for (const place of eachInstructionLValue(instr)) {
+        validate(place);
+      }
+      for (const place of eachInstructionOperand(instr)) {
+        validate(place);
+      }
+    }
+    for (const place of eachTerminalOperand(block.terminal)) {
+      validate(place);
+    }
+  }
+}
 
 /**
  * This pass assumes that all program blocks are properly nested with respect to fallthroughs

compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts

@@ -1270,6 +1270,10 @@ export type AbstractValue = {
   context: ReadonlySet<Place>;
 };
 
+export function isRangeMutable(range: MutableRange): boolean {
+  return range.end > range.start + 1;
+}
+
 /**
  * The reason for the kind of a value.
  */

compiler/packages/babel-plugin-react-compiler/src/HIR/MergeOverlappingReactiveScopesHIR.ts

@@ -3,6 +3,7 @@ import {
   InstructionId,
   Place,
   ReactiveScope,
+  isRangeMutable,
   makeInstructionId,
 } from ".";
 import { getPlaceScope } from "../ReactiveScopes/BuildReactiveBlocks";
@@ -124,6 +125,9 @@ export function mergeOverlappingReactiveScopesHIR(fn: HIRFunction): void {
     const nextScope = joinedScopes.find(originalScope);
     if (nextScope !== null && nextScope !== originalScope) {
       place.identifier.scope = nextScope;
+      if (isRangeMutable(place.identifier.mutableRange)) {
+        place.identifier.mutableRange = nextScope.range;
+      }
     }
   }
 }

compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts

@@ -35,7 +35,7 @@ import type {
   Terminal,
   Type,
 } from "./HIR";
-import { GotoVariant, InstructionKind } from "./HIR";
+import { GotoVariant, InstructionKind, isRangeMutable } from "./HIR";
 
 export type Options = {
   indent: number;
@@ -716,10 +716,6 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
   return value;
 }
 
-function isMutable(range: MutableRange): boolean {
-  return range.end > range.start + 1;
-}
-
 const DEBUG_MUTABLE_RANGES = false;
 function printMutableRange(identifier: Identifier): string {
   if (DEBUG_MUTABLE_RANGES) {
@@ -732,11 +728,11 @@ function printMutableRange(identifier: Identifier): string {
     ) {
       return `[${range.start}:${range.end}] scope=[${scopeRange.start}:${scopeRange.end}]`;
     }
-    return isMutable(range) ? `[${range.start}:${range.end}]` : "";
+    return isRangeMutable(range) ? `[${range.start}:${range.end}]` : "";
   }
   // in non-debug mode, prefer the scope range if it exists
   const range = identifier.scope?.range ?? identifier.mutableRange;
-  return isMutable(range) ? `[${range.start}:${range.end}]` : "";
+  return isRangeMutable(range) ? `[${range.start}:${range.end}]` : "";
 }
 
 export function printLValue(lval: LValue): string {

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignMethodCallScopes.ts

@@ -9,6 +9,7 @@ import {
   HIRFunction,
   IdentifierId,
   ReactiveScope,
+  isRangeMutable,
   makeInstructionId,
 } from "../HIR";
 import DisjointSet from "../Utils/DisjointSet";
@@ -69,10 +70,19 @@ export function alignMethodCallScopes(fn: HIRFunction): void {
       const mappedScope = scopeMapping.get(instr.lvalue.identifier.id);
       if (mappedScope !== undefined) {
         instr.lvalue.identifier.scope = mappedScope;
+        if (
+          mappedScope != null &&
+          isRangeMutable(instr.lvalue.identifier.mutableRange)
+        ) {
+          instr.lvalue.identifier.mutableRange = mappedScope.range;
+        }
       } else if (instr.lvalue.identifier.scope !== null) {
         const mergedScope = mergedScopes.find(instr.lvalue.identifier.scope);
         if (mergedScope != null) {
           instr.lvalue.identifier.scope = mergedScope;
+          if (isRangeMutable(instr.lvalue.identifier.mutableRange)) {
+            instr.lvalue.identifier.mutableRange = mergedScope.range;
+          }
         }
       }
     }

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts

@@ -11,6 +11,7 @@ import {
   HIRFunction,
   Identifier,
   ReactiveScope,
+  isRangeMutable,
   makeInstructionId,
 } from "../HIR";
 import { eachInstructionValueOperand } from "../HIR/visitors";
@@ -93,6 +94,9 @@ export function alignObjectMethodScopes(fn: HIRFunction): void {
         const root = scopeGroupsMap.get(identifier.scope);
         if (root != null) {
           identifier.scope = root;
+          if (isRangeMutable(identifier.mutableRange)) {
+            identifier.mutableRange = root.range;
+          }
         }
         // otherwise, this identifier's scope was not affected by this pass
       }

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts

@@ -7,10 +7,12 @@
 
 import {
   HIRFunction,
+  Identifier,
   IdentifierId,
   makeInstructionId,
   Place,
   ReactiveValue,
+  isRangeMutable,
 } from "../HIR";
 import { eachReactiveValueOperand } from "./visitors";
 
@@ -134,6 +136,9 @@ function visit(
          */
         for (const operand of eachReactiveValueOperand(value)) {
           operand.identifier.scope = fbtScope;
+          if (isRangeMutable(operand.identifier.mutableRange)) {
+            operand.identifier.mutableRange = fbtScope.range;
+          }
 
           // Expand the jsx element's range to account for its operands
           fbtScope.range.start = makeInstructionId(
@@ -167,6 +172,9 @@ function visit(
             continue;
           }
           operand.identifier.scope = fbtScope;
+          if (isRangeMutable(operand.identifier.mutableRange)) {
+            operand.identifier.mutableRange = fbtScope.range;
+          }
 
           // Expand the jsx element's range to account for its operands
           fbtScope.range.start = makeInstructionId(

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-fbt-preserve-whitespace-param.expect.md

@@ -0,0 +1,100 @@
+
+## Input
+
+```javascript
+import fbt from "fbt";
+
+/**
+ * Currently fails with the following:
+ * Found differences in evaluator results
+ *   Non-forget (expected):
+ *   (kind: ok) <div><span>Jason !</span></div>
+ *   Forget:
+ *   (kind: ok) <div><span>Jason!</span></div>
+
+ */
+
+function Foo(props) {
+  return (
+    // prettier-ignore
+    <div>
+      <fbt desc={"Dialog to show to user"}>
+        <span>
+          <fbt:param name="user name">
+            {props.name}
+          </fbt:param>
+        !
+        </span>
+      </fbt>
+    </div>
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{ name: "Jason" }],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import fbt from "fbt";
+
+/**
+ * Currently fails with the following:
+ * Found differences in evaluator results
+ *   Non-forget (expected):
+ *   (kind: ok) <div><span>Jason !</span></div>
+ *   Forget:
+ *   (kind: ok) <div><span>Jason!</span></div>
+
+ */
+
+function Foo(props) {
+  const $ = _c(2);
+  let t0;
+  if ($[0] !== props.name) {
+    t0 = (
+      <div>
+        {fbt._(
+          "{=m0}",
+          [
+            fbt._implicitParam(
+              "=m0",
+              <span>
+                {fbt._(
+                  "{user name}!",
+                  [
+                    fbt._param(
+                      "user name",
+
+                      props.name,
+                    ),
+                  ],
+                  { hk: "mBBZ9" },
+                )}
+              </span>,
+            ),
+          ],
+          { hk: "3RVfuk" },
+        )}
+      </div>
+    );
+    $[0] = props.name;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{ name: "Jason" }],
+};
+
+```
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/bug-fbt-preserve-whitespace-param.tsx

@@ -0,0 +1,32 @@
+import fbt from "fbt";
+
+/**
+ * Currently fails with the following:
+ * Found differences in evaluator results
+ *   Non-forget (expected):
+ *   (kind: ok) <div><span>Jason !</span></div>
+ *   Forget:
+ *   (kind: ok) <div><span>Jason!</span></div>
+
+ */
+
+function Foo(props) {
+  return (
+    // prettier-ignore
+    <div>
+      <fbt desc={"Dialog to show to user"}>
+        <span>
+          <fbt:param name="user name">
+            {props.name}
+          </fbt:param>
+        !
+        </span>
+      </fbt>
+    </div>
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{ name: "Jason" }],
+};

compiler/packages/snap/src/SproutTodoFilter.ts

@@ -484,6 +484,7 @@ const skipFilter = new Set([
   "rules-of-hooks/rules-of-hooks-69521d94fa03",
 
   // bugs
+  "fbt/bug-fbt-preserve-whitespace-param",
   "bug-invalid-hoisting-functionexpr",
   "original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block",
   "original-reactive-scopes-fork/bug-hoisted-declaration-with-scope",