Skip to content

Commit 493d360

Browse files
committed
feat: Remove conditional flattening - all helpers now use StackExpression
- Modified ResolvedCallExpression to always use flattening - Removed helper functions for checking nested calls (no longer needed) - Updated StackOperation type to temporarily allow any TupleExpression - Added fallback in compileStackOperation to handle unflatted expressions This creates a mixed system where StackExpression can contain both: - Flattened operations (PushConstant, CallHelper, etc.) - Nested expressions (Curry, Not, etc.) that delegate back to expr() Next steps: Incrementally add flattening for each expression type
1 parent 8ec65e8 commit 493d360

File tree

3 files changed

+14
-46
lines changed
  • packages/@glimmer

3 files changed

+14
-46
lines changed

packages/@glimmer/compiler/lib/passes/2-encoding/expressions.ts

Lines changed: 6 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -236,36 +236,6 @@ export function callArgs(
236236
return encodeCallArgs(positionalArgs, namedArgs, { insertAtPrefix: false });
237237
}
238238

239-
// Helper to check if any arguments contain nested helper calls
240-
function hasNestedHelperCalls(args: mir.Args): boolean {
241-
// Check positional arguments
242-
if (args.positional.list.isPresent) {
243-
for (const arg of args.positional.list.toArray()) {
244-
if (isNestedHelperCall(arg)) {
245-
return true;
246-
}
247-
}
248-
}
249-
250-
// Check named arguments
251-
if (args.named.entries.isPresent) {
252-
for (const entry of args.named.entries.toArray()) {
253-
if (isNestedHelperCall(view.get(entry.value))) {
254-
return true;
255-
}
256-
}
257-
}
258-
259-
return false;
260-
}
261-
262-
function isNestedHelperCall(expr: mir.ExpressionNode | ASTv2.UnresolvedBinding): boolean {
263-
return expr.type === 'ResolvedCallExpression' || expr.type === 'CallExpression';
264-
}
265-
266-
function shouldFlattenExpression(expr: mir.ResolvedCallExpression): boolean {
267-
return hasNestedHelperCalls(expr.args);
268-
}
269239

270240
function flattenResolvedCallExpression(
271241
expr: mir.ResolvedCallExpression
@@ -372,9 +342,9 @@ function flattenExpression(
372342
) {
373343
operations.push(encoded as WireFormat.Expressions.StackOperation);
374344
} else {
375-
// For complex expressions we don't support flattening yet,
376-
// fall back to not flattening
377-
throw new Error(`Cannot flatten expression type with opcode ${opcode}`);
345+
// For expressions we don't support flattening yet (like Curry, Not, etc.),
346+
// push the encoded expression as-is. The opcode compiler will handle it.
347+
operations.push(encoded as WireFormat.Expressions.StackOperation);
378348
}
379349
} else {
380350
// It's a literal value, push it as a constant
@@ -481,17 +451,9 @@ function InterpolateExpression({
481451

482452
function ResolvedCallExpression(
483453
expr: mir.ResolvedCallExpression
484-
): WireFormat.Expressions.SomeCallHelper | WireFormat.Expressions.StackExpression {
485-
// Check if we should flatten this expression
486-
if (shouldFlattenExpression(expr)) {
487-
return flattenResolvedCallExpression(expr);
488-
}
489-
490-
return [
491-
Op.CallResolved,
492-
view.get(expr.callee).symbol,
493-
callArgs(expr.args.positional, expr.args.named),
494-
];
454+
): WireFormat.Expressions.StackExpression {
455+
// Always flatten all helper expressions for unified handling
456+
return flattenResolvedCallExpression(expr);
495457
}
496458

497459
function CallExpression({

packages/@glimmer/interfaces/lib/compile/wire-format/api.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ export namespace Expressions {
232232
export type Undefined = [UndefinedOpcode];
233233

234234
// Operations that can appear in a flattened stack expression
235+
// TODO: This is transitional - eventually only flat operations will be allowed
235236
export type StackOperation =
236237
| PushImmediate
237238
| PushConstant
@@ -244,7 +245,9 @@ export namespace Expressions {
244245
| GetLexicalSymbol
245246
| GetKeyword
246247
| GetDynamicVar
247-
| Undefined;
248+
| Undefined
249+
// Temporarily allow any expression while we transition to full flattening
250+
| TupleExpression;
248251

249252
// For now, keep existing GetPath for backward compatibility
250253
// and add new flattened form

packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/expr.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ function compileStackOperation(
129129
}
130130

131131
default:
132-
exhausted(operation);
132+
// For nested expressions that we haven't flattened yet (Curry, Not, etc.),
133+
// delegate back to expr() to handle them. This is temporary - once all
134+
// expressions are flattened, this case will be removed.
135+
expr(encode, operation as WireFormat.Expression);
133136
}
134137
}
135138

0 commit comments

Comments
 (0)