compiler: Fixtures to show challenge with setters

I quickly experimented with enabling support for get/set syntax. This confirmed my suspicion that these won't be safe without additional work. The "set" fixture example shows how we assume PropertyStore is only a Store effect on the object, but with a setter it needs to be modeled as an arbitrary mutation. If we were to support getters and setters I think we'd need to: * Support `this` syntax (maybe limited to just within object methods) * Enforce that getters cannot mutate any free variables, cannot mutate `this` * Enforce that setters cannot mutate any free variables (but allow mutating `this`). In other words, enforce that you're using getters/setters in a reasonable way that matches our modeling (get is a read, set is a store to the object). [ghstack-poisoned]
2025-11-01 09:12:30 +00:00 · 2024-05-25 23:19:33 +01:00
parent 9bd33c975c
commit 6d75e612fe
11 changed files with 220 additions and 94 deletions
@@ -1386,7 +1386,8 @@ function lowerStatement(

 function lowerObjectMethod(
  builder: HIRBuilder,
-  property: NodePath<t.ObjectMethod>
+  property: NodePath<t.ObjectMethod>,
+  kind: t.ObjectMethod["kind"]
 ): InstructionValue {
  const loc = property.node.loc ?? GeneratedSource;
  const loweredFunc = lowerFunction(builder, property);
@@ -1397,6 +1398,7 @@ function lowerObjectMethod(
  return {
    kind: "ObjectMethod",
    loc,
+    methodKind: kind,
    loweredFunc,
  };
 }
@@ -1520,16 +1522,11 @@ function lowerExpression(
            place,
          });
        } else if (propertyPath.isObjectMethod()) {
-          if (propertyPath.node.kind !== "method") {
-            builder.errors.push({
-              reason: `(BuildHIR::lowerExpression) Handle ${propertyPath.node.kind} functions in ObjectExpression`,
-              severity: ErrorSeverity.Todo,
-              loc: propertyPath.node.loc ?? null,
-              suggestions: null,
-            });
-            continue;
-          }
-          const method = lowerObjectMethod(builder, propertyPath);
+          const method = lowerObjectMethod(
+            builder,
+            propertyPath,
+            propertyPath.node.kind
+          );
          const place = lowerValueToTemporary(builder, method);
          const loweredKey = lowerObjectPropertyKey(builder, propertyPath);
          if (!loweredKey) {
@@ -686,6 +686,7 @@ export type LoweredFunction = {
 export type ObjectMethod = {
  kind: "ObjectMethod";
  loc: SourceLocation;
+  methodKind: "method" | "get" | "set";
  loweredFunc: LoweredFunction;
 };

@@ -1617,7 +1617,7 @@ function codegenInstructionValue(
               * https://github.com/babel/babel/blob/v7.7.4/packages/babel-types/src/definitions/core.js#L599-L603
               */
              const babelNode = t.objectMethod(
-                "method",
+                method.methodKind,
                key,
                fn.params,
                fn.body,
@@ -1,39 +0,0 @@
-
-## Input
-
-```javascript
-function Component({ value }) {
-  const object = {
-    get value() {
-      return value;
-    },
-  };
-  return <div>{object.value}</div>;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: foo,
-  params: [{ value: 0 }],
-  sequentialRenders: [{ value: 1 }, { value: 2 }],
-};
-
-```
-
-
-## Error
-
-```
-  1 | function Component({ value }) {
-  2 |   const object = {
-> 3 |     get value() {
-    |     ^^^^^^^^^^^^^
-> 4 |       return value;
-    | ^^^^^^^^^^^^^^^^^^^
-> 5 |     },
-    | ^^^^^^ Todo: (BuildHIR::lowerExpression) Handle get functions in ObjectExpression (3:5)
-  6 |   };
-  7 |   return <div>{object.value}</div>;
-  8 | }
-```
-          
-      
@@ -1,41 +0,0 @@
-
-## Input
-
-```javascript
-function Component(props) {
-  let value;
-  const object = {
-    set value(v) {
-      value = v;
-    },
-  };
-  object.value = props.value;
-  return <div>{value}</div>;
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: foo,
-  params: [{ value: 0 }],
-  sequentialRenders: [{ value: 1 }, { value: 2 }],
-};
-
-```
-
-
-## Error
-
-```
-  2 |   let value;
-  3 |   const object = {
-> 4 |     set value(v) {
-    |     ^^^^^^^^^^^^^^
-> 5 |       value = v;
-    | ^^^^^^^^^^^^^^^^
-> 6 |     },
-    | ^^^^^^ Todo: (BuildHIR::lowerExpression) Handle set functions in ObjectExpression (4:6)
-  7 |   };
-  8 |   object.value = props.value;
-  9 |   return <div>{value}</div>;
-```
-          
-      
@@ -0,0 +1,63 @@
+
+## Input
+
+```javascript
+function Component({ value }) {
+  const object = {
+    get value() {
+      return value;
+    },
+  };
+  return <div>{object.value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  sequentialRenders: [{ value: 1 }, { value: 2 }],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(t0) {
+  const $ = _c(4);
+  const { value } = t0;
+  let t1;
+  if ($[0] !== value) {
+    t1 = {
+      get value() {
+        return value;
+      },
+    };
+    $[0] = value;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  const object = t1;
+  let t2;
+  if ($[2] !== object.value) {
+    t2 = <div>{object.value}</div>;
+    $[2] = object.value;
+    $[3] = t2;
+  } else {
+    t2 = $[3];
+  }
+  return t2;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  sequentialRenders: [{ value: 1 }, { value: 2 }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>1</div>
+<div>2</div>
@@ -8,7 +8,7 @@ function Component({ value }) {
 }

 export const FIXTURE_ENTRYPOINT = {
-  fn: foo,
+  fn: Component,
  params: [{ value: 0 }],
  sequentialRenders: [{ value: 1 }, { value: 2 }],
 };
@@ -0,0 +1,63 @@
+
+## Input
+
+```javascript
+function Component(props) {
+  let value;
+  const object = {
+    set value(v) {
+      value = v;
+    },
+  };
+  object.value = props.value;
+  return <div>{value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  // sequentialRenders: [{ value: 1 }, { value: 2 }],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(props) {
+  const $ = _c(3);
+  let value;
+  if ($[0] !== props.value) {
+    const object = {
+      set value(v) {
+        value = v;
+      },
+    };
+
+    object.value = props.value;
+    $[0] = props.value;
+    $[1] = value;
+  } else {
+    value = $[1];
+  }
+  let t0;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = <div>{value}</div>;
+    $[2] = t0;
+  } else {
+    t0 = $[2];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  // sequentialRenders: [{ value: 1 }, { value: 2 }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>0</div>
@@ -10,7 +10,7 @@ function Component(props) {
 }

 export const FIXTURE_ENTRYPOINT = {
-  fn: foo,
+  fn: Component,
  params: [{ value: 0 }],
  sequentialRenders: [{ value: 1 }, { value: 2 }],
 };
@@ -0,0 +1,66 @@
+
+## Input
+
+```javascript
+function Component(props) {
+  let value;
+  const object = {
+    setValue(v) {
+      value = v;
+    },
+  };
+  object.setValue(props.value);
+  return <div>{value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  sequentialRenders: [{ value: 1 }, { value: 2 }],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(props) {
+  const $ = _c(4);
+  let value;
+  if ($[0] !== props.value) {
+    const object = {
+      setValue(v) {
+        value = v;
+      },
+    };
+
+    object.setValue(props.value);
+    $[0] = props.value;
+    $[1] = value;
+  } else {
+    value = $[1];
+  }
+  const t0 = value;
+  let t1;
+  if ($[2] !== t0) {
+    t1 = <div>{t0}</div>;
+    $[2] = t0;
+    $[3] = t1;
+  } else {
+    t1 = $[3];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  sequentialRenders: [{ value: 1 }, { value: 2 }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>1</div>
+<div>2</div>
@@ -0,0 +1,16 @@
+function Component(props) {
+  let value;
+  const object = {
+    setValue(v) {
+      value = v;
+    },
+  };
+  object.setValue(props.value);
+  return <div>{value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ value: 0 }],
+  sequentialRenders: [{ value: 1 }, { value: 2 }],
+};