From 2fa04d128e47abb27645a90873b08369e82c9ee7 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 18 Sep 2023 16:42:25 -0700 Subject: [PATCH] Transitive mixed type, take 2 This is a redo of #1640 now that we've established the necessary infrastructure, most notably `Effect.ConditionallyMutate` and `noAlias` from #2103 earlier in this stack. We can now understand the semantics of hooks that return deeply readonly values composed of primitives, arrays, or objects such that any `.map()` or `.filter()` calls are guaranteed to be the corresponding array methods. That further allows us to refine, since we know that the lambdas passed to these calls can't alias, are conditionally mutable, etc. All in all this should let us memoize less in practice. --- .../src/HIR/Environment.ts | 28 ++++- .../src/HIR/ObjectShape.ts | 37 ++++++ ...and-local-variables-with-default.expect.md | 117 ++++++++++-------- ...ed-scope-declarations-and-locals.expect.md | 61 +++++---- .../compiler/optional-call-logical.expect.md | 22 +++- ...ject-method-calls-mutable-lambda.expect.md | 62 ++++++++++ ...only-object-method-calls-mutable-lambda.js | 16 +++ .../readonly-object-method-calls.expect.md | 64 ++++++++++ .../compiler/readonly-object-method-calls.js | 14 +++ ...epro-scope-missing-mutable-range.expect.md | 29 +++-- .../tagged-template-in-hook.expect.md | 11 +- .../fixture-test-utils/src/compiler-utils.ts | 12 ++ .../packages/sprout/src/SproutTodoFilter.ts | 2 + 13 files changed, 377 insertions(+), 98 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.js create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.js diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts index 21c6b1c761..ec82bbfc34 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts @@ -28,6 +28,7 @@ import { makeIdentifierId, } from "./HIR"; import { + BuiltInMixedReadonlyId, DefaultMutatingHook, DefaultNonmutatingHook, FunctionSignature, @@ -55,6 +56,25 @@ export type Hook = { * compiler to avoid memoizing arguments. */ noAlias?: boolean; + + /** + * Specifies whether the hook returns data that is composed of: + * - undefined + * - null + * - boolean + * - number + * - string + * - arrays whose items are also transitiveMixed + * - objects whose values are also transitiveMixed + * + * Many state management and data-fetching APIs return data that meets + * this criteria since this is JSON + undefined. Forget can compile + * hooks that return transitively mixed data more optimally because it + * can make inferences about some method calls (especially array methods + * like `data.items.map(...)` since these builtin types have few built-in + * methods. + */ + transitiveMixedData?: boolean; }; // TODO(mofeiZ): User defined global types (with corresponding shapes). @@ -236,7 +256,9 @@ export class Environment { addHook(this.#shapes, [], { positionalParams: [], restParam: hook.effectKind, - returnType: { kind: "Poly" }, + returnType: hook.transitiveMixedData + ? { kind: "Object", shapeId: BuiltInMixedReadonlyId } + : { kind: "Poly" }, returnValueKind: hook.valueKind, calleeEffect: Effect.Read, hookKind: "Custom", @@ -312,7 +334,9 @@ export class Environment { loc: null, suggestions: null, }); - return shape.properties.get(property) ?? null; + return ( + shape.properties.get(property) ?? shape.properties.get("*") ?? null + ); } else { return null; } diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts index 66e1c94845..ec7f50a0c0 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/ObjectShape.ts @@ -170,6 +170,7 @@ export const BuiltInUseStateId = "BuiltInUseState"; export const BuiltInSetStateId = "BuiltInSetState"; export const BuiltInUseRefId = "BuiltInUseRefId"; export const BuiltInRefValueId = "BuiltInRefValue"; +export const BuiltInMixedReadonlyId = "BuiltInMixedReadonly"; /** * ShapeRegistry with default definitions for built-ins. @@ -294,6 +295,42 @@ addObject(BUILTIN_SHAPES, BuiltInUseRefId, [ addObject(BUILTIN_SHAPES, BuiltInRefValueId, []); +addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ + [ + "toString", + addFunction(BUILTIN_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: PRIMITIVE_TYPE, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Immutable, + }), + ], + [ + "map", + addFunction(BUILTIN_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: { kind: "Object", shapeId: BuiltInArrayId }, + calleeEffect: Effect.ConditionallyMutate, + returnValueKind: ValueKind.Mutable, + noAlias: true, + }), + ], + [ + "filter", + addFunction(BUILTIN_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: { kind: "Object", shapeId: BuiltInArrayId }, + calleeEffect: Effect.ConditionallyMutate, + returnValueKind: ValueKind.Mutable, + noAlias: true, + }), + ], + ["*", { kind: "Object", shapeId: BuiltInMixedReadonlyId }], +]); + export const DefaultMutatingHook = addHook(BUILTIN_SHAPES, [], { positionalParams: [], restParam: Effect.ConditionallyMutate, diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md index f12061aa0a..247056d02b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-and-local-variables-with-default.expect.md @@ -56,87 +56,94 @@ function useFragment(_arg1, _arg2) { } function Component(props) { - const $ = useMemoCache(16); - const post = useFragment(graphql`...`, props.post); - const c_0 = $[0] !== post; + const $ = useMemoCache(17); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = graphql`...`; + $[0] = t0; + } else { + t0 = $[0]; + } + const post = useFragment(t0, props.post); + const c_1 = $[1] !== post; let media; let allUrls; let onClick; - if (c_0) { + if (c_1) { allUrls = []; - const { media: t0, comments: t2, urls: t4 } = post; - const c_4 = $[4] !== t0; - let t1; - if (c_4) { - t1 = t0 === undefined ? null : t0; - $[4] = t0; + const { media: t1, comments: t3, urls: t5 } = post; + const c_5 = $[5] !== t1; + let t2; + if (c_5) { + t2 = t1 === undefined ? null : t1; $[5] = t1; - } else { - t1 = $[5]; - } - media = t1; - const c_6 = $[6] !== t2; - let t3; - if (c_6) { - t3 = t2 === undefined ? [] : t2; $[6] = t2; + } else { + t2 = $[6]; + } + media = t2; + const c_7 = $[7] !== t3; + let t4; + if (c_7) { + t4 = t3 === undefined ? [] : t3; $[7] = t3; - } else { - t3 = $[7]; - } - const comments = t3; - const c_8 = $[8] !== t4; - let t5; - if (c_8) { - t5 = t4 === undefined ? [] : t4; $[8] = t4; - $[9] = t5; } else { - t5 = $[9]; + t4 = $[8]; } - const urls = t5; - const c_10 = $[10] !== comments.length; + const comments = t4; + const c_9 = $[9] !== t5; let t6; - if (c_10) { - t6 = (e) => { + if (c_9) { + t6 = t5 === undefined ? [] : t5; + $[9] = t5; + $[10] = t6; + } else { + t6 = $[10]; + } + const urls = t6; + const c_11 = $[11] !== comments.length; + let t7; + if (c_11) { + t7 = (e) => { if (!comments.length) { return; } console.log(comments.length); }; - $[10] = comments.length; - $[11] = t6; + $[11] = comments.length; + $[12] = t7; } else { - t6 = $[11]; + t7 = $[12]; } - onClick = t6; + onClick = t7; allUrls.push(...urls); - $[0] = post; - $[1] = media; - $[2] = allUrls; - $[3] = onClick; + $[1] = post; + $[2] = media; + $[3] = allUrls; + $[4] = onClick; } else { - media = $[1]; - allUrls = $[2]; - onClick = $[3]; + media = $[2]; + allUrls = $[3]; + onClick = $[4]; } - const c_12 = $[12] !== media; - const c_13 = $[13] !== allUrls; - const c_14 = $[14] !== onClick; - let t7; - if (c_12 || c_13 || c_14) { - t7 = ; - $[12] = media; - $[13] = allUrls; - $[14] = onClick; - $[15] = t7; + const c_13 = $[13] !== media; + const c_14 = $[14] !== allUrls; + const c_15 = $[15] !== onClick; + let t8; + if (c_13 || c_14 || c_15) { + t8 = ; + $[13] = media; + $[14] = allUrls; + $[15] = onClick; + $[16] = t8; } else { - t7 = $[15]; + t8 = $[16]; } - return t7; + return t8; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md index 9d606ddc5b..bf056a0292 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructuring-mixed-scope-declarations-and-locals.expect.md @@ -30,53 +30,60 @@ function Component(props) { ```javascript import { unstable_useMemoCache as useMemoCache } from "react"; function Component(props) { - const $ = useMemoCache(8); - const post = useFragment(graphql`...`, props.post); - const c_0 = $[0] !== post; + const $ = useMemoCache(9); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = graphql`...`; + $[0] = t0; + } else { + t0 = $[0]; + } + const post = useFragment(t0, props.post); + const c_1 = $[1] !== post; let media; let onClick; - if (c_0) { + if (c_1) { const allUrls = []; const { media: t83, comments, urls } = post; media = t83; - const c_3 = $[3] !== comments.length; - let t0; - if (c_3) { - t0 = (e) => { + const c_4 = $[4] !== comments.length; + let t1; + if (c_4) { + t1 = (e) => { if (!comments.length) { return; } console.log(comments.length); }; - $[3] = comments.length; - $[4] = t0; + $[4] = comments.length; + $[5] = t1; } else { - t0 = $[4]; + t1 = $[5]; } - onClick = t0; + onClick = t1; allUrls.push(...urls); - $[0] = post; - $[1] = media; - $[2] = onClick; + $[1] = post; + $[2] = media; + $[3] = onClick; } else { - media = $[1]; - onClick = $[2]; + media = $[2]; + onClick = $[3]; } - const c_5 = $[5] !== media; - const c_6 = $[6] !== onClick; - let t1; - if (c_5 || c_6) { - t1 = ; - $[5] = media; - $[6] = onClick; - $[7] = t1; + const c_6 = $[6] !== media; + const c_7 = $[7] !== onClick; + let t2; + if (c_6 || c_7) { + t2 = ; + $[6] = media; + $[7] = onClick; + $[8] = t2; } else { - t1 = $[7]; + t2 = $[8]; } - return t1; + return t2; } ``` diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/optional-call-logical.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/optional-call-logical.expect.md index f0b8d40ff3..3166d8eaf3 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/optional-call-logical.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/optional-call-logical.expect.md @@ -12,9 +12,27 @@ function Component(props) { ## Code ```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; function Component(props) { - const item = useFragment(graphql`...`, props.item); - return item.items?.map((item_0) => renderItem(item_0)) ?? []; + const $ = useMemoCache(3); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = graphql`...`; + $[0] = t0; + } else { + t0 = $[0]; + } + const item = useFragment(t0, props.item); + const c_1 = $[1] !== item.items; + let t1; + if (c_1) { + t1 = item.items?.map((item_0) => renderItem(item_0)) ?? []; + $[1] = item.items; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; } ``` diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md new file mode 100644 index 0000000000..e3c6f16f68 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.expect.md @@ -0,0 +1,62 @@ + +## Input + +```javascript +// @enableNoAliasOptimizations +function Component(props) { + const x = makeObject(); + const user = useFragment( + graphql`fragment Component_user on User { ... }`, + props.user + ); + const posts = user.timeline.posts.edges.nodes.map((node) => { + x.y = true; + return ; + }); + posts.push({}); + const count = posts.length; + foo(count); + return <>{posts}; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableNoAliasOptimizations +function Component(props) { + const $ = useMemoCache(3); + const x = makeObject(); + const user = useFragment( + graphql`fragment Component_user on User { ... }`, + props.user + ); + const posts = user.timeline.posts.edges.nodes.map((node) => { + x.y = true; + return ; + }); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[0] = t0; + } else { + t0 = $[0]; + } + posts.push(t0); + const count = posts.length; + foo(count); + const c_1 = $[1] !== posts; + let t1; + if (c_1) { + t1 = <>{posts}; + $[1] = posts; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.js new file mode 100644 index 0000000000..0bd543d9d3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls-mutable-lambda.js @@ -0,0 +1,16 @@ +// @enableNoAliasOptimizations +function Component(props) { + const x = makeObject(); + const user = useFragment( + graphql`fragment Component_user on User { ... }`, + props.user + ); + const posts = user.timeline.posts.edges.nodes.map((node) => { + x.y = true; + return ; + }); + posts.push({}); + const count = posts.length; + foo(count); + return <>{posts}; +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md new file mode 100644 index 0000000000..0bf19c9303 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +// @enableNoAliasOptimizations +function Component(props) { + const user = useFragment( + graphql`fragment Component_user on User { ... }`, + props.user + ); + const posts = user.timeline.posts.edges.nodes.map((node) => ( + + )); + posts.push({}); + const count = posts.length; + foo(count); + return <>{posts}; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; // @enableNoAliasOptimizations +function Component(props) { + const $ = useMemoCache(5); + const user = useFragment( + graphql`fragment Component_user on User { ... }`, + props.user + ); + const c_0 = $[0] !== user.timeline.posts.edges.nodes; + let posts; + if (c_0) { + posts = user.timeline.posts.edges.nodes.map((node) => ); + let t0; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[2] = t0; + } else { + t0 = $[2]; + } + posts.push(t0); + $[0] = user.timeline.posts.edges.nodes; + $[1] = posts; + } else { + posts = $[1]; + } + const count = posts.length; + foo(count); + const c_3 = $[3] !== posts; + let t1; + if (c_3) { + t1 = <>{posts}; + $[3] = posts; + $[4] = t1; + } else { + t1 = $[4]; + } + return t1; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.js new file mode 100644 index 0000000000..b8e03b19be --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/readonly-object-method-calls.js @@ -0,0 +1,14 @@ +// @enableNoAliasOptimizations +function Component(props) { + const user = useFragment( + graphql`fragment Component_user on User { ... }`, + props.user + ); + const posts = user.timeline.posts.edges.nodes.map((node) => ( + + )); + posts.push({}); + const count = posts.length; + foo(count); + return <>{posts}; +} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md index 3014745db0..d5cd24eeac 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md @@ -20,20 +20,27 @@ function HomeDiscoStoreItemTileRating(props) { ```javascript import { unstable_useMemoCache as useMemoCache } from "react"; function HomeDiscoStoreItemTileRating(props) { - const $ = useMemoCache(1); + const $ = useMemoCache(3); const item = useFragment(); + const c_0 = $[0] !== item; let count; - count = 0; - const aggregates = item?.aggregates || []; - aggregates.forEach((aggregate) => { - count = count + (aggregate.count || 0); - }); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = {count}; - $[0] = t0; + if (c_0) { + count = 0; + const aggregates = item?.aggregates || []; + aggregates.forEach((aggregate) => { + count = count + (aggregate.count || 0); + }); + $[0] = item; + $[1] = count; } else { - t0 = $[0]; + count = $[1]; + } + let t0; + if ($[2] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {count}; + $[2] = t0; + } else { + t0 = $[2]; } return t0; } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/tagged-template-in-hook.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/tagged-template-in-hook.expect.md index a7db7906f2..fbfaee47a8 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/tagged-template-in-hook.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/tagged-template-in-hook.expect.md @@ -12,8 +12,17 @@ function Component(props) { ## Code ```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; function Component(props) { - const user = useFragment(graphql`fragment on User { name }`, props.user); + const $ = useMemoCache(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = graphql`fragment on User { name }`; + $[0] = t0; + } else { + t0 = $[0]; + } + const user = useFragment(t0, props.user); return user.name; } diff --git a/compiler/packages/fixture-test-utils/src/compiler-utils.ts b/compiler/packages/fixture-test-utils/src/compiler-utils.ts index f8d2d3d46b..a904e0f03b 100644 --- a/compiler/packages/fixture-test-utils/src/compiler-utils.ts +++ b/compiler/packages/fixture-test-utils/src/compiler-utils.ts @@ -100,6 +100,17 @@ export function transformFixtureInput( { valueKind: "frozen" as ValueKind, effectKind: "freeze" as Effect, + transitiveMixedData: false, + noAlias: false, + }, + ], + [ + "useFragment", + { + valueKind: "frozen" as ValueKind, + effectKind: "freeze" as Effect, + transitiveMixedData: true, + noAlias: true, }, ], [ @@ -107,6 +118,7 @@ export function transformFixtureInput( { valueKind: "mutable" as ValueKind, effectKind: "read" as Effect, + transitiveMixedData: false, noAlias: true, }, ], diff --git a/compiler/packages/sprout/src/SproutTodoFilter.ts b/compiler/packages/sprout/src/SproutTodoFilter.ts index 9ff3b3e406..142e2e9d85 100644 --- a/compiler/packages/sprout/src/SproutTodoFilter.ts +++ b/compiler/packages/sprout/src/SproutTodoFilter.ts @@ -413,6 +413,8 @@ const skipFilter = new Set([ "fbt-template-string-same-scope", "component-declaration-basic.flow", "nested-function-with-param-as-captured-dep", + "readonly-object-method-calls", + "readonly-object-method-calls-mutable-lambda", // TODO: 🌲 "forest-basic",