@samitouri / QOS-React-1 / commits / 52c6a34da3

[be] Limit scope of temporaries in codegen

This morning @mofeiZ reminded me that our codegen doesn't really have any guards against reordering, since temporaries are lazily emitted. We're relying on the fact that our lowering and memoization carefully preserves order of evaluation, such that delaying the instructions in codegen doesn't change semantics. To help catch any mistakes with this, I had previously added code that reset the codegen context's temporaries before/after exiting a reactive scope. That ensured that temporaries from within the scope weren't accessible outside it. This PR extends that approach to _all_ blocks, so that temporaries created within a block aren't accessible outside it. I'm also going to explore more actively resetting temporaries after they "should" be used. There are a couple cases where temporaries are reused, though, which we have to change first.

Joe Savona committed Oct 10, 2023 at 13:15 UTC 52c6a34da30ffee665183df2aa8f97e3511fbd9e
1 file changed +31 -1
compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+31 -1
@@ -139,6 +139,36 @@ class Context {
139 }
140
141 function codegenBlock(cx: Context, block: ReactiveBlock): t.BlockStatement {
142 + const temp = new Map(cx.temp);
143 + const result = codegenBlockNoReset(cx, block);
144 + // Check that the block only added new temporaries and did not update the
145 + // value of any existing temporary
146 + for (const [key, value] of cx.temp) {
147 + if (!temp.has(key)) {
148 + continue;
149 + }
150 + CompilerError.invariant(temp.get(key)! === value, {
151 + loc: null,
152 + reason: "Expected temporary value to be unchanged",
153 + description: null,
154 + suggestions: null,
155 + });
156 + }
157 + cx.temp = temp;
158 + return result;
159 +}
160 +
161 +/**
162 + * Generates code for the block, without resetting the Context's temporary state.
163 + * This should not be used unless it is expected that temporaries from this block
164 + * can be referenced later, which is currently only true for sequence expressions
165 + * where the final `value` is expected to reference the temporary created in the
166 + * preceding instructions of the sequence.
167 + */
168 +function codegenBlockNoReset(
169 + cx: Context,
170 + block: ReactiveBlock
171 +): t.BlockStatement {
172 const statements: Array<t.Statement> = [];
173 for (const item of block) {
174 switch (item.kind) {
@@ -1282,7 +1312,7 @@ function codegenInstructionValue(
1312 break;
1313 }
1314 case "SequenceExpression": {
1285 - const body = codegenBlock(
1315 + const body = codegenBlockNoReset(
1316 cx,
1317 instrValue.instructions.map((instruction) => ({
1318 kind: "instruction",