-
Notifications
You must be signed in to change notification settings - Fork 48.8k
[compiler][bugfix] Fix hoisting of let declarations #32724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
let bar; | ||
bar = _temp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could rewrite these to be a StoreLocal LValue: {kind: "Let"}
which would result in cleaner output. Note that we currently produce a DeclareContext
followed by a StoreContext
for all non-hoisted context variables.
I implemented this with a DeclareContext
followed by StoreContext
to better preserve instruction semantics (a StoreLocal
followed by StoreContext
to the same variable feels a bit nonsensical). Since this is currently the last pass before codegen, this isn't a problem at the moment.
(Found when compiling Meta React code) Let variable declarations and reassignments are currently rewritten to `StoreLocal <varName>` instructions, which each translates to a new `const varName` declaration in codegen. ```js // Example input function useHook() { const getX = () => x; let x = CONSTANT1; if (cond) { x += CONSTANT2; } return <Stringify getX={getX} /> } // Compiled output, prior to this PR import { c as _c } from "react/compiler-runtime"; function useHook() { const $ = _c(1); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const getX = () => x; let x = CONSTANT1; if (cond) { let x = x + CONSTANT2; x; } t0 = <Stringify getX={getX} />; $[0] = t0; } else { t0 = $[0]; } return t0; } ``` This also manifests as a babel internal error when replacing the original function declaration with the compiler output. The below compilation output fails with `Duplicate declaration "x" (This is an error on an internal node. Probably an internal error.)`. ```js // example input let x = CONSTANT1; if (cond) { x += CONSTANT2; x = CONSTANT3; } // current output let x = CONSTANT1; if (playheadDragState) { let x = x + CONSTANT2 x; let x = CONSTANT3; } ```
|
(Found when compiling Meta React code) Let variable declarations and reassignments are currently rewritten to `StoreLocal <varName>` instructions, which each translates to a new `const varName` declaration in codegen. ```js // Example input function useHook() { const getX = () => x; let x = CONSTANT1; if (cond) { x += CONSTANT2; } return <Stringify getX={getX} /> } // Compiled output, prior to this PR import { c as _c } from "react/compiler-runtime"; function useHook() { const $ = _c(1); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { const getX = () => x; let x = CONSTANT1; if (cond) { let x = x + CONSTANT2; x; } t0 = <Stringify getX={getX} />; $[0] = t0; } else { t0 = $[0]; } return t0; } ``` This also manifests as a babel internal error when replacing the original function declaration with the compiler output. The below compilation output fails with `Duplicate declaration "x" (This is an error on an internal node. Probably an internal error.)`. ```js // example input let x = CONSTANT1; if (cond) { x += CONSTANT2; x = CONSTANT3; } // current output let x = CONSTANT1; if (playheadDragState) { let x = x + CONSTANT2 x; let x = CONSTANT3; } ``` DiffTrain build for [254dc4d](254dc4d)
(Found when compiling Meta React code)
Let variable declarations and reassignments are currently rewritten to
StoreLocal <varName>
instructions, which each translates to a newconst varName
declaration in codegen.This also manifests as a babel internal error when replacing the original function declaration with the compiler output. The below compilation output fails with
Duplicate declaration "x" (This is an error on an internal node. Probably an internal error.)
.