From d4fba739ba32b66acc409680972bbebaf4d0a895 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 18 Nov 2024 11:17:55 -0500 Subject: [PATCH] [compiler] repro for type inference + control flow bug Repro for bug in our type inference system. We currently propagate inferred types through control flow / potential type guards. Note that this is inconsistent with both [Flow](https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkO4AX34kA0SRWxgABAAxYjsgC87OAAB0oOzReythU2Mh2YKQNyILLeMKxeymrgZNLhCIbsL6QBuYVs7DsgBCVD5AuVYolUClMpAZsoiqtorVGvZWpuSqg9OFMAeyjg0HZdTmW3lAAp5NKAPJoABWcwkAEppWZGLg4O12fJ2bSuTyhSKxSwJEJKCKAOQ2tiVvMi3MAMkbOasNb5vP5svlsoNPuFfoD8JFGQqUel8vZAB9TVReCHoHa0MRnlBUwWIJbi6K4DB2RHbGxk1uVSrd-uAIShsDh4hR5PHoun5-siS1SgQADuHuw34AotQECUBGsqysmfYvuyvrbqepblg2EFitBKpwRWOZ9vSuQgA0JgkEGUBJBk9gmCA9JAA) and [Typescript](https://www.typescriptlang.org/play/?#code/C4TwDgpgBAYg9nKBeKBvAUFLUDWBLAOwBMAuKAInjnIBpNsA3AQwBsBXCMgtgWwCMIAJ3QBfANzpQkKACEmg5GnpZ8xMuTmDayqM3aco3fkLoj0AMzYEAxsDxwCUawAsI1nFQAUADzJw+AFZuwACUZEwAzhFCwBFQ3lB4cVRK2InmUJ4AhJ4A5KpEuYmOCQBkpfEAdAXISCiUCOQhIalp2MDOgnAA7oYQvQCigl2CnuRWEN6QthBETTpmZhZWtvaOPEyEPmQpAD6y8jRODqRQfAgsEEwEYbAIrVh4GZ7WJy0Ybdgubh4IPiEST5YQQQYBsQQlQHYMxpEFgiHxCQiIA) --- .../bug-type-inference-control-flow.expect.md | 114 ++++++++++++++++++ .../bug-type-inference-control-flow.ts | 41 +++++++ .../packages/snap/src/SproutTodoFilter.ts | 1 + .../snap/src/sprout/shared-runtime.ts | 3 +- 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.expect.md new file mode 100644 index 0000000000000..8fc049fa6f915 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.expect.md @@ -0,0 +1,114 @@ + +## Input + +```javascript +import {arrayPush, CONST_NUMBER0, mutate} from 'shared-runtime'; + +/** + * Repro for bug in our type inference system. We currently propagate inferred + * types through control flow / potential type guards. Note that this is + * inconsistent with both Flow and Typescript. + * https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkO4AX34kA0SRWxgABAAxYjsgC87OAAB0oOzReythU2Mh2YKQNyILLeMKxeymrgZNLhCIbsL6QBuYVs7DsgBCVD5AuVYolUClMpAZsoiqtorVGvZWpuSqg9OFMAeyjg0HZdTmW3lAAp5NKAPJoABWcwkAEppWZGLg4O12fJ2bSuTyhSKxSwJEJKCKAOQ2tiVvMi3MAMkbOasNb5vP5svlsoNPuFfoD8JFGQqUel8vZAB9TVReCHoHa0MRnlBUwWIJbi6K4DB2RHbGxk1uVSrd-uAIShsDh4hR5PHoun5-siS1SgQADuHuw34AotQECUBGsqysmfYvuyvrbqepblg2EFitBKpwRWOZ9vSuQgA0JgkEGUBJBk9gmCA9JAA + * https://www.typescriptlang.org/play/?#code/C4TwDgpgBAYg9nKBeKBvAUFLUDWBLAOwBMAuKAInjnIBpNsA3AQwBsBXCMgtgWwCMIAJ3QBfANzpQkKACEmg5GnpZ8xMuTmDayqM3aco3fkLoj0AMzYEAxsDxwCUawAsI1nFQAUADzJw+AFZuwACUZEwAzhFCwBFQ3lB4cVRK2InmUJ4AhJ4A5KpEuYmOCQBkpfEAdAXISCiUCOQhIalp2MDOgnAA7oYQvQCigl2CnuRWEN6QthBETTpmZhZWtvaOPEyEPmQpAD6y8jRODqRQfAgsEEwEYbAIrVh4GZ7WJy0Ybdgubh4IPiEST5YQQQYBsQQlQHYMxpEFgiHxCQiIA + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * [2] + * [3] + * Forget: + * (kind: ok) + * [2] + * [2,3] + */ +function useFoo({cond, value}: {cond: boolean; value: number}) { + const x = {value: cond ? CONST_NUMBER0 : []}; + mutate(x); + + const xValue = x.value; + let result; + if (typeof xValue === 'number') { + result = xValue + 1; // (1) here we infer xValue is a primitive + } else { + result = arrayPush(xValue, value); // (2) and propagate it to all other xValue references + } + + return result; +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{cond: true}], + sequentialRenders: [ + {cond: false, value: 2}, + {cond: false, value: 3}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { arrayPush, CONST_NUMBER0, mutate } from "shared-runtime"; + +/** + * Repro for bug in our type inference system. We currently propagate inferred + * types through control flow / potential type guards. Note that this is + * inconsistent with both Flow and Typescript. + * https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkO4AX34kA0SRWxgABAAxYjsgC87OAAB0oOzReythU2Mh2YKQNyILLeMKxeymrgZNLhCIbsL6QBuYVs7DsgBCVD5AuVYolUClMpAZsoiqtorVGvZWpuSqg9OFMAeyjg0HZdTmW3lAAp5NKAPJoABWcwkAEppWZGLg4O12fJ2bSuTyhSKxSwJEJKCKAOQ2tiVvMi3MAMkbOasNb5vP5svlsoNPuFfoD8JFGQqUel8vZAB9TVReCHoHa0MRnlBUwWIJbi6K4DB2RHbGxk1uVSrd-uAIShsDh4hR5PHoun5-siS1SgQADuHuw34AotQECUBGsqysmfYvuyvrbqepblg2EFitBKpwRWOZ9vSuQgA0JgkEGUBJBk9gmCA9JAA + * https://www.typescriptlang.org/play/?#code/C4TwDgpgBAYg9nKBeKBvAUFLUDWBLAOwBMAuKAInjnIBpNsA3AQwBsBXCMgtgWwCMIAJ3QBfANzpQkKACEmg5GnpZ8xMuTmDayqM3aco3fkLoj0AMzYEAxsDxwCUawAsI1nFQAUADzJw+AFZuwACUZEwAzhFCwBFQ3lB4cVRK2InmUJ4AhJ4A5KpEuYmOCQBkpfEAdAXISCiUCOQhIalp2MDOgnAA7oYQvQCigl2CnuRWEN6QthBETTpmZhZWtvaOPEyEPmQpAD6y8jRODqRQfAgsEEwEYbAIrVh4GZ7WJy0Ybdgubh4IPiEST5YQQQYBsQQlQHYMxpEFgiHxCQiIA + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * [2] + * [3] + * Forget: + * (kind: ok) + * [2] + * [2,3] + */ +function useFoo(t0) { + const $ = _c(5); + const { cond, value } = t0; + let x; + if ($[0] !== cond) { + x = { value: cond ? CONST_NUMBER0 : [] }; + mutate(x); + $[0] = cond; + $[1] = x; + } else { + x = $[1]; + } + + const xValue = x.value; + let result; + if (typeof xValue === "number") { + result = xValue + 1; + } else { + let t1; + if ($[2] !== value || $[3] !== xValue) { + t1 = arrayPush(xValue, value); + $[2] = value; + $[3] = xValue; + $[4] = t1; + } else { + t1 = $[4]; + } + result = t1; + } + return result; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ cond: true }], + sequentialRenders: [ + { cond: false, value: 2 }, + { cond: false, value: 3 }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.ts new file mode 100644 index 0000000000000..4b8957dc8d277 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-type-inference-control-flow.ts @@ -0,0 +1,41 @@ +import {arrayPush, CONST_NUMBER0, mutate} from 'shared-runtime'; + +/** + * Repro for bug in our type inference system. We currently propagate inferred + * types through control flow / potential type guards. Note that this is + * inconsistent with both Flow and Typescript. + * https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkO4AX34kA0SRWxgABAAxYjsgC87OAAB0oOzReythU2Mh2YKQNyILLeMKxeymrgZNLhCIbsL6QBuYVs7DsgBCVD5AuVYolUClMpAZsoiqtorVGvZWpuSqg9OFMAeyjg0HZdTmW3lAAp5NKAPJoABWcwkAEppWZGLg4O12fJ2bSuTyhSKxSwJEJKCKAOQ2tiVvMi3MAMkbOasNb5vP5svlsoNPuFfoD8JFGQqUel8vZAB9TVReCHoHa0MRnlBUwWIJbi6K4DB2RHbGxk1uVSrd-uAIShsDh4hR5PHoun5-siS1SgQADuHuw34AotQECUBGsqysmfYvuyvrbqepblg2EFitBKpwRWOZ9vSuQgA0JgkEGUBJBk9gmCA9JAA + * https://www.typescriptlang.org/play/?#code/C4TwDgpgBAYg9nKBeKBvAUFLUDWBLAOwBMAuKAInjnIBpNsA3AQwBsBXCMgtgWwCMIAJ3QBfANzpQkKACEmg5GnpZ8xMuTmDayqM3aco3fkLoj0AMzYEAxsDxwCUawAsI1nFQAUADzJw+AFZuwACUZEwAzhFCwBFQ3lB4cVRK2InmUJ4AhJ4A5KpEuYmOCQBkpfEAdAXISCiUCOQhIalp2MDOgnAA7oYQvQCigl2CnuRWEN6QthBETTpmZhZWtvaOPEyEPmQpAD6y8jRODqRQfAgsEEwEYbAIrVh4GZ7WJy0Ybdgubh4IPiEST5YQQQYBsQQlQHYMxpEFgiHxCQiIA + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * [2] + * [3] + * Forget: + * (kind: ok) + * [2] + * [2,3] + */ +function useFoo({cond, value}: {cond: boolean; value: number}) { + const x = {value: cond ? CONST_NUMBER0 : []}; + mutate(x); + + const xValue = x.value; + let result; + if (typeof xValue === 'number') { + result = xValue + 1; // (1) here we infer xValue is a primitive + } else { + result = arrayPush(xValue, value); // (2) and propagate it to all other xValue references + } + + return result; +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{cond: true}], + sequentialRenders: [ + {cond: false, value: 2}, + {cond: false, value: 3}, + ], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index f1f5ef0f6b3c7..bb51ece12f28e 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -485,6 +485,7 @@ const skipFilter = new Set([ 'bug-aliased-capture-mutate', 'bug-functiondecl-hoisting', 'bug-try-catch-maybe-null-dependency', + 'bug-type-inference-control-flow', 'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted', 'bug-invalid-phi-as-dependency', 'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond', diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index e6e82d6b77e05..bd069ae6d0299 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -107,8 +107,9 @@ export function setPropertyByKey< return arg; } -export function arrayPush(arr: Array, ...values: Array): void { +export function arrayPush(arr: Array, ...values: Array): Array { arr.push(...values); + return arr; } export function graphql(value: string): string {