From 495c808f7a5c38d2194ebb1e5cbdddab5757f0a5 Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Wed, 11 Mar 2020 10:54:42 -0700 Subject: [PATCH 1/2] WIP fix template size regression --- .../compiler/lib/builder-interface.ts | 11 +-- packages/@glimmer/compiler/lib/builder.ts | 81 +++++++++++++---- .../@glimmer/compiler/lib/compiler-ops.ts | 2 - .../compiler/lib/javascript-compiler.ts | 42 +++------ .../compiler/lib/template-compiler.ts | 21 +++-- .../compiler/lib/wire-format-debug.ts | 61 ++++++++----- .../interfaces/lib/compile/wire-format.d.ts | 88 +++++++++++++------ .../opcode-compiler/lib/syntax/expressions.ts | 71 +++++++++++---- .../opcode-compiler/lib/syntax/macros.ts | 27 ++---- .../opcode-compiler/lib/syntax/statements.ts | 4 +- .../@glimmer/opcode-compiler/lib/utils.ts | 41 ++++++--- 11 files changed, 288 insertions(+), 161 deletions(-) diff --git a/packages/@glimmer/compiler/lib/builder-interface.ts b/packages/@glimmer/compiler/lib/builder-interface.ts index 99ced435d4..1b66345879 100644 --- a/packages/@glimmer/compiler/lib/builder-interface.ts +++ b/packages/@glimmer/compiler/lib/builder-interface.ts @@ -1,4 +1,4 @@ -import { Dict, Option, ExpressionContext } from '@glimmer/interfaces'; +import { Dict, Option } from '@glimmer/interfaces'; import { dict, assertNever, expect } from '@glimmer/util'; export type BuilderParams = BuilderExpression[]; @@ -106,7 +106,7 @@ export type NormalizedStatement = export function normalizeStatement(statement: BuilderStatement): NormalizedStatement { if (Array.isArray(statement)) { if (statementIsExpression(statement)) { - return normalizeAppendExpression(statement, ExpressionContext.AppendSingleId); + return normalizeAppendExpression(statement); } else if (isSugaryArrayStatement(statement)) { return normalizeSugaryArrayStatement(statement); } else { @@ -223,11 +223,7 @@ function normalizeVerboseStatement(statement: VerboseStatement): NormalizedState } case Builder.Append: { - return normalizeAppendExpression( - statement[1], - ExpressionContext.AppendSingleId, - statement[2] - ); + return normalizeAppendExpression(statement[1], statement[2]); } case Builder.Modifier: { @@ -588,7 +584,6 @@ export type NormalizedExpression = export function normalizeAppendExpression( expression: BuilderExpression, - _context: ExpressionContext, forceTrusted = false ): AppendExpr | AppendPath { if (expression === null || expression === undefined) { diff --git a/packages/@glimmer/compiler/lib/builder.ts b/packages/@glimmer/compiler/lib/builder.ts index 5be3e908b5..15a4c09093 100644 --- a/packages/@glimmer/compiler/lib/builder.ts +++ b/packages/@glimmer/compiler/lib/builder.ts @@ -1,7 +1,7 @@ import { WireFormat, Option, Dict, Expressions, ExpressionContext } from '@glimmer/interfaces'; import Op = WireFormat.SexpOpcodes; -import { dict, assertNever, assert, values } from '@glimmer/util'; +import { dict, assertNever, assert, values, exhausted } from '@glimmer/util'; import { BuilderStatement, BuilderComment, @@ -192,8 +192,6 @@ export function buildStatement( [ Op.Append, +normalized.trusted, - 0, - 0, buildPath(normalized.path, ExpressionContext.AppendSingleId, symbols), ], ]; @@ -204,8 +202,6 @@ export function buildStatement( [ Op.Append, +normalized.trusted, - 0, - 0, buildExpression(normalized.expr, ExpressionContext.Expression, symbols), ], ]; @@ -217,11 +213,11 @@ export function buildStatement( let builtHash: WireFormat.Core.Hash = hash ? buildHash(hash, symbols) : null; let builtExpr: WireFormat.Expression = buildPath(path, ExpressionContext.CallHead, symbols); - return [[Op.Append, +trusted, 0, 0, [Op.Call, 0, 0, builtExpr, builtParams, builtHash]]]; + return [[Op.Append, +trusted, [Op.Call, builtExpr, builtParams, builtHash]]]; } case HeadKind.Literal: { - return [[Op.Append, 1, 0, 0, normalized.value]]; + return [[Op.Append, 1, normalized.value]]; } case HeadKind.Comment: { @@ -443,7 +439,7 @@ export function buildExpression( let builtHash = buildHash(expr.hash, symbols); let builtExpr = buildPath(expr.path, ExpressionContext.CallHead, symbols); - return [Op.Call, 0, 0, builtExpr, builtParams, builtHash]; + return [Op.Call, builtExpr, builtParams, builtHash]; } case ExpressionKind.HasBlock: { @@ -477,34 +473,87 @@ export function buildExpression( } } } + export function buildPath( path: Path, context: ExpressionContext, symbols: Symbols ): Expressions.GetPath { if (path.tail.length === 0) { - return [Op.GetPath, buildVar(path.variable, context, symbols), path.tail]; + return buildVar(path.variable, context, symbols, path.tail); } else { - return [Op.GetPath, buildVar(path.variable, ExpressionContext.Expression, symbols), path.tail]; + return buildVar(path.variable, ExpressionContext.Expression, symbols, path.tail); } } +export function buildVar( + head: Variable, + context: ExpressionContext, + symbols: Symbols, + path: string[] +): Expressions.GetPath; export function buildVar( head: Variable, context: ExpressionContext, symbols: Symbols -): Expressions.GetSymbol | Expressions.GetFree | Expressions.GetContextualFree { +): Expressions.Get; +export function buildVar( + head: Variable, + context: ExpressionContext, + symbols: Symbols, + path?: string[] +): Expressions.GetPath | Expressions.Get { + let op: Expressions.Get[0] = Op.GetSymbol; + let sym: number; switch (head.kind) { case VariableKind.Free: - return [Op.GetContextualFree, symbols.freeVar(head.name), context]; + op = expressionContextOp(context); + sym = symbols.freeVar(head.name); + break; + default: + op = Op.GetSymbol; + sym = getSymbolForVar(head.kind, symbols, head.name); + } + return (path === undefined ? [op, sym] : [op, sym, path]) as + | Expressions.Get + | Expressions.GetPath; +} + +function getSymbolForVar( + kind: Exclude, + symbols: Symbols, + name: string +) { + switch (kind) { case VariableKind.Arg: - return [Op.GetSymbol, symbols.arg(head.name)]; + return symbols.arg(name); case VariableKind.Block: - return [Op.GetSymbol, symbols.block(head.name)]; + return symbols.block(name); case VariableKind.Local: - return [Op.GetSymbol, symbols.local(head.name)]; + return symbols.local(name); case VariableKind.This: - return [Op.GetSymbol, symbols.this()]; + return symbols.this(); + default: + return exhausted(kind); + } +} + +export function expressionContextOp(context: ExpressionContext) { + switch (context) { + case ExpressionContext.AppendSingleId: + return Op.GetFreeInAppendSingleId; + case ExpressionContext.Expression: + return Op.GetFreeInExpression; + case ExpressionContext.CallHead: + return Op.GetFreeInCallHead; + case ExpressionContext.BlockHead: + return Op.GetFreeInBlockHead; + case ExpressionContext.ModifierHead: + return Op.GetFreeInModifierHead; + case ExpressionContext.ComponentHead: + return Op.GetFreeInComponentHead; + default: + return exhausted(context); } } diff --git a/packages/@glimmer/compiler/lib/compiler-ops.ts b/packages/@glimmer/compiler/lib/compiler-ops.ts index edddc58cd1..dec127764d 100644 --- a/packages/@glimmer/compiler/lib/compiler-ops.ts +++ b/packages/@glimmer/compiler/lib/compiler-ops.ts @@ -26,8 +26,6 @@ export interface InputOps { comment: [AST.CommentStatement]; } -// type Location = ['loc', [null, [number, number], [number, number]]]; - export interface AllocateSymbolsOps { startProgram: AST.Template; endProgram: void; diff --git a/packages/@glimmer/compiler/lib/javascript-compiler.ts b/packages/@glimmer/compiler/lib/javascript-compiler.ts index 0b4980ca45..a1ce9b4f87 100644 --- a/packages/@glimmer/compiler/lib/javascript-compiler.ts +++ b/packages/@glimmer/compiler/lib/javascript-compiler.ts @@ -15,6 +15,7 @@ import { Expressions, ExpressionContext, } from '@glimmer/interfaces'; +import { expressionContextOp } from './builder'; export type str = string; import Core = WireFormat.Core; @@ -236,11 +237,11 @@ export default class JavaScriptCompiler implements Processor()]); + this.push([SexpOpcodes.Append, +trusted, this.popValue()]); } comment(value: string) { @@ -251,7 +252,7 @@ export default class JavaScriptCompiler implements Processor(); let params = this.popValue(); let hash = this.popValue(); - this.push([SexpOpcodes.Modifier, 0, 0, name, params, hash]); + this.push([SexpOpcodes.Modifier, name, params, hash]); } block([template, inverse]: [number, Option]) { @@ -424,9 +425,9 @@ export default class JavaScriptCompiler implements Processor(); - this.pushValue([SexpOpcodes.GetPath, head, rest]); + getPath(path: string[]) { + let [op, sym] = this.popValue(); + this.pushValue([op, sym, path]); } getSymbol(head: number) { @@ -438,7 +439,7 @@ export default class JavaScriptCompiler implements Processor([SexpOpcodes.GetContextualFree, head, context]); + this.pushValue([expressionContextOp(context), head]); } concat() { @@ -446,18 +447,11 @@ export default class JavaScriptCompiler implements Processor(); + let { value: head } = this.popLocatedValue(); let params = this.popValue(); let hash = this.popValue(); - this.pushValue([ - SexpOpcodes.Call, - start(location), - end(location), - head, - params, - hash, - ]); + this.pushValue([SexpOpcodes.Call, head, params, hash]); } /// Stack Management Opcodes @@ -537,19 +531,3 @@ export default class JavaScriptCompiler implements Processor().value; } } - -function start(location: Option): number { - if (location) { - return location.start; - } else { - return -1; - } -} - -function end(location: Option): number { - if (location) { - return location.end - location.start; - } else { - return -1; - } -} diff --git a/packages/@glimmer/compiler/lib/template-compiler.ts b/packages/@glimmer/compiler/lib/template-compiler.ts index e3fb757840..a7eccc8c30 100644 --- a/packages/@glimmer/compiler/lib/template-compiler.ts +++ b/packages/@glimmer/compiler/lib/template-compiler.ts @@ -261,17 +261,23 @@ export default class TemplateCompiler implements Processor { private argPath(head: string, rest: string[], loc: AST.BaseNode) { this.opcode(['getArg', head], loc); - this.opcode(['getPath', rest], loc); + if (rest.length > 0) { + this.opcode(['getPath', rest], loc); + } } private varPath(head: string, rest: string[], context: ExpressionContext, loc: AST.BaseNode) { this.opcode(['getVar', [head, context]], loc); - this.opcode(['getPath', rest], loc); + if (rest.length > 0) { + this.opcode(['getPath', rest], loc); + } } private thisPath(rest: string[], loc: AST.BaseNode) { this.opcode(['getThis'], loc); - this.opcode(['getPath', rest], loc); + if (rest.length > 0) { + this.opcode(['getPath', rest], loc); + } } private expression(path: AST.Expression, context: ExpressionContext, expr: AST.Node) { @@ -339,14 +345,13 @@ export default class TemplateCompiler implements Processor { } private path(expr: AST.PathExpression, context: ExpressionContext) { - let [head, ...rest] = expr.parts; - + let { parts } = expr; if (expr.data) { - this.argPath(`@${head}`, rest, expr); + this.argPath(`@${parts[0]}`, parts.slice(1), expr); } else if (expr.this) { - this.thisPath(expr.parts, expr); + this.thisPath(parts, expr); } else { - this.varPath(head, rest, context, expr); + this.varPath(parts[0], parts.slice(1), context, expr); } } diff --git a/packages/@glimmer/compiler/lib/wire-format-debug.ts b/packages/@glimmer/compiler/lib/wire-format-debug.ts index c74337dd09..79af940dff 100644 --- a/packages/@glimmer/compiler/lib/wire-format-debug.ts +++ b/packages/@glimmer/compiler/lib/wire-format-debug.ts @@ -5,7 +5,7 @@ import { SerializedInlineBlock, SerializedTemplateBlock, } from '@glimmer/interfaces'; -import { dict, assertNever } from '@glimmer/util'; +import { dict, exhausted } from '@glimmer/util'; export default class WireFormatDebugger { constructor(private program: SerializedTemplateBlock, _parameters?: number[]) {} @@ -87,8 +87,8 @@ export default class WireFormatDebugger { return [ 'modifier', this.formatOpcode(opcode[1]), - this.formatParams(opcode[4]), - this.formatHash(opcode[5]), + this.formatParams(opcode[2]), + this.formatHash(opcode[3]), ]; case Op.Component: @@ -109,18 +109,6 @@ export default class WireFormatDebugger { // this.formatBlocks(opcode[4]), // ]; - case Op.GetSymbol: - return ['get-symbol', this.program.symbols[opcode[1]], opcode[1]]; - - case Op.GetFree: - return ['get-free', this.program.upvars[opcode[1]]]; - - case Op.GetContextualFree: - return ['get-contextual-free', this.program.upvars[opcode[1]], opcode[2]]; - - case Op.GetPath: - return ['get-path', this.formatOpcode(opcode[1]), opcode[2]]; - case Op.HasBlock: return ['has-block', opcode[1]]; @@ -133,17 +121,50 @@ export default class WireFormatDebugger { case Op.Call: return [ 'call', - this.formatOpcode(opcode[3]), - this.formatParams(opcode[4]), - this.formatHash(opcode[5]), + this.formatOpcode(opcode[1]), + this.formatParams(opcode[2]), + this.formatHash(opcode[3]), ]; case Op.Concat: return ['concat', this.formatParams(opcode[1] as WireFormat.Core.Params)]; default: { - let opName = opcode[0]; - throw assertNever(opName, `unexpected ${opName}`); + let [op, sym, path] = opcode; + let opName: string; + let varName: string; + if (op === Op.GetSymbol) { + varName = this.program.symbols[sym]; + opName = 'get-symbol'; + } else { + varName = this.program.upvars[sym]; + switch (op) { + case Op.GetFree: + opName = 'get-free'; + break; + case Op.GetFreeInAppendSingleId: + opName = 'get-free-in-append-single-id'; + break; + case Op.GetFreeInBlockHead: + opName = 'get-free-in-block-head'; + break; + case Op.GetFreeInCallHead: + opName = 'get-free-in-call-head'; + break; + case Op.GetFreeInComponentHead: + opName = 'get-free-in-component-head'; + break; + case Op.GetFreeInExpression: + opName = 'get-free-in-expression'; + break; + case Op.GetFreeInModifierHead: + opName = 'get-free-in-modifier-head'; + break; + default: + return exhausted(op); + } + } + return path ? [opName, varName, path] : [opName, varName]; } } } else { diff --git a/packages/@glimmer/interfaces/lib/compile/wire-format.d.ts b/packages/@glimmer/interfaces/lib/compile/wire-format.d.ts index 2023e6892c..e63f807561 100644 --- a/packages/@glimmer/interfaces/lib/compile/wire-format.d.ts +++ b/packages/@glimmer/interfaces/lib/compile/wire-format.d.ts @@ -38,15 +38,24 @@ export const enum SexpOpcodes { // Expressions - GetSymbol = 24, - GetFree = 25, - GetContextualFree = 26, - GetPath = 27, - HasBlock = 28, - HasBlockParams = 29, - Undefined = 30, - Call = 31, - Concat = 32, + HasBlock = 24, + HasBlockParams = 25, + Undefined = 26, + Call = 27, + Concat = 28, + + // GetPath + GetSymbol = 32, // GetPath + 0-2, + GetFree = 33, + GetFreeInAppendSingleId = 34, // GetContextualFree + 0-5 + GetFreeInExpression = 35, + GetFreeInCallHead = 36, + GetFreeInBlockHead = 37, + GetFreeInModifierHead = 38, + GetFreeInComponentHead = 39, + + GetPathStart = GetSymbol, + GetContextualFreeStart = GetFreeInAppendSingleId, } export type StatementSexpOpcode = Statement[0]; @@ -76,19 +85,19 @@ export namespace Core { export const enum ExpressionContext { // An `Append` is a single identifier that is contained inside a curly (either in a // content curly or an attribute curly) - AppendSingleId = 'AppendSingleId', + AppendSingleId = 0, // An `Expression` is evaluated into a value (e.g. `person.name` in `(call person.name)` // or `person.name` in `@name={{person.name}}`). This represents a syntactic position // that must evaluate as an expression by virtue of its position in the syntax. - Expression = 'Expression', + Expression = 1, // A `CallHead` is the head of an expression that is definitely a call - CallHead = 'CallHead', + CallHead = 2, // A `BlockHead` is the head of an expression that is definitely a block - BlockHead = 'BlockHead', + BlockHead = 3, // A `ModifierHead` is the head of an expression that is definitely a modifir - ModifierHead = 'ModifierHead', + ModifierHead = 4, // A `ComponentHead` is the head of an expression that is definitely a component - ComponentHead = 'ComponentHead', + ComponentHead = 5, } export namespace Expressions { @@ -98,16 +107,45 @@ export namespace Expressions { export type GetSymbol = [SexpOpcodes.GetSymbol, number]; export type GetFree = [SexpOpcodes.GetFree, number]; - export type GetContextualFree = [SexpOpcodes.GetContextualFree, number, ExpressionContext]; - export type GetPath = [SexpOpcodes.GetPath, Get, Path]; + export type GetFreeInAppendSingleId = [SexpOpcodes.GetFreeInAppendSingleId, number]; + export type GetFreeInExpression = [SexpOpcodes.GetFreeInExpression, number]; + export type GetFreeInCallHead = [SexpOpcodes.GetFreeInCallHead, number]; + export type GetFreeInBlockHead = [SexpOpcodes.GetFreeInBlockHead, number]; + export type GetFreeInModifierHead = [SexpOpcodes.GetFreeInModifierHead, number]; + export type GetFreeInComponentHead = [SexpOpcodes.GetFreeInComponentHead, number]; + + export type GetContextualFree = + | GetFreeInAppendSingleId + | GetFreeInExpression + | GetFreeInCallHead + | GetFreeInBlockHead + | GetFreeInModifierHead + | GetFreeInComponentHead; + export type Get = GetSymbol | GetFree | GetContextualFree; + + export type GetPathSymbol = [SexpOpcodes.GetSymbol, number, Path]; + export type GetPathFree = [SexpOpcodes.GetFree, number, Path]; + export type GetPathFreeInAppendSingleId = [SexpOpcodes.GetFreeInAppendSingleId, number, Path]; + export type GetPathFreeInExpression = [SexpOpcodes.GetFreeInExpression, number, Path]; + export type GetPathFreeInCallHead = [SexpOpcodes.GetFreeInCallHead, number, Path]; + export type GetPathFreeInBlockHead = [SexpOpcodes.GetFreeInBlockHead, number, Path]; + export type GetPathFreeInModifierHead = [SexpOpcodes.GetFreeInModifierHead, number, Path]; + export type GetPathFreeInComponentHead = [SexpOpcodes.GetFreeInComponentHead, number, Path]; + + export type GetPathContextualFree = + | GetPathFreeInAppendSingleId + | GetPathFreeInExpression + | GetPathFreeInCallHead + | GetPathFreeInBlockHead + | GetPathFreeInModifierHead + | GetPathFreeInComponentHead; + export type GetPath = GetPathSymbol | GetPathFree | GetPathContextualFree; export type Value = string | number | boolean | null; export type Undefined = [SexpOpcodes.Undefined]; export type TupleExpression = - | GetSymbol - | GetFree - | GetContextualFree + | Get | GetPath | Concat | HasBlock @@ -116,13 +154,11 @@ export namespace Expressions { | Undefined; export type Expression = TupleExpression | Value; - export type Get = GetSymbol | GetFree | GetContextualFree; type Recursive = T; export interface Concat extends Recursive<[SexpOpcodes.Concat, Core.ConcatParams]> {} - export interface Helper - extends Recursive<[SexpOpcodes.Call, number, number, Expression, Option, Hash]> {} + export interface Helper extends Recursive<[SexpOpcodes.Call, Expression, Option, Hash]> {} export interface HasBlock extends Recursive<[SexpOpcodes.HasBlock, Expression]> {} export interface HasBlockParams extends Recursive<[SexpOpcodes.HasBlockParams, Expression]> {} } @@ -139,11 +175,9 @@ export namespace Statements { export type Blocks = Core.Blocks; export type Path = Core.Path; - // Statement = [op, flags, start, offset, ...args] - - export type Append = [SexpOpcodes.Append, number, number, number, Expression]; + export type Append = [SexpOpcodes.Append, number, Expression]; export type Comment = [SexpOpcodes.Comment, string]; - export type Modifier = [SexpOpcodes.Modifier, number, number, Expression, Params, Hash]; + export type Modifier = [SexpOpcodes.Modifier, Expression, Params, Hash]; export type Block = [SexpOpcodes.Block, Expression, Option, Hash, Blocks]; export type Component = [SexpOpcodes.Component, Expression, Attribute[], Hash, Blocks]; export type OpenElement = [SexpOpcodes.OpenElement, string, boolean]; diff --git a/packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts b/packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts index 627aa36706..65eb792cf2 100644 --- a/packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts +++ b/packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts @@ -7,6 +7,7 @@ import { ExpressionSexpOpcode, ExpressionCompileActions, ContainingMetadata, + ExpressionContext, } from '@glimmer/interfaces'; import { op } from '../opcode-builder/encoder'; import { Call, PushPrimitiveReference } from '../opcode-builder/helpers/vm'; @@ -27,8 +28,11 @@ EXPRESSIONS.add(SexpOpcodes.Concat, ([, parts]) => { return out; }); -EXPRESSIONS.add(SexpOpcodes.Call, ([, start, offset, name, params, hash], meta) => { +EXPRESSIONS.add(SexpOpcodes.Call, ([, name, params, hash], meta) => { // TODO: triage this in the WF compiler + let start = 0; + let offset = 0; + if (isComponent(name, meta)) { if (!params || params.length === 0) { return op('Error', { @@ -67,19 +71,21 @@ EXPRESSIONS.add(SexpOpcodes.Call, ([, start, offset, name, params, hash], meta) }); }); +function isGetContextualFree( + opcode: Expressions.TupleExpression +): opcode is Expressions.GetContextualFree { + return opcode[0] >= SexpOpcodes.GetContextualFreeStart; +} + function isComponent(expr: Expressions.Expression, meta: ContainingMetadata): boolean { if (!Array.isArray(expr)) { return false; } - if (expr[0] === SexpOpcodes.GetPath) { + if (isGetContextualFree(expr)) { let head = expr[1]; - if ( - head[0] === SexpOpcodes.GetContextualFree && - meta.upvars && - meta.upvars[head[1]] === 'component' - ) { + if (meta.upvars && meta.upvars[head] === 'component') { return true; } else { return false; @@ -89,17 +95,52 @@ function isComponent(expr: Expressions.Expression, meta: ContainingMetadata): bo return false; } -EXPRESSIONS.add(SexpOpcodes.GetSymbol, ([, head]) => [op(Op.GetVariable, head)]); +EXPRESSIONS.add(SexpOpcodes.GetSymbol, ([, sym, path]) => withPath(op(Op.GetVariable, sym), path)); +EXPRESSIONS.add(SexpOpcodes.GetFree, ([, sym, path]) => withPath(op('ResolveFree', sym), path)); +EXPRESSIONS.add(SexpOpcodes.GetFreeInAppendSingleId, ([, sym, path]) => + withPath( + op('ResolveContextualFree', { freeVar: sym, context: ExpressionContext.AppendSingleId }), + path + ) +); +EXPRESSIONS.add(SexpOpcodes.GetFreeInExpression, ([, sym, path]) => + withPath( + op('ResolveContextualFree', { freeVar: sym, context: ExpressionContext.Expression }), + path + ) +); +EXPRESSIONS.add(SexpOpcodes.GetFreeInCallHead, ([, sym, path]) => + withPath(op('ResolveContextualFree', { freeVar: sym, context: ExpressionContext.CallHead }), path) +); +EXPRESSIONS.add(SexpOpcodes.GetFreeInBlockHead, ([, sym, path]) => + withPath( + op('ResolveContextualFree', { freeVar: sym, context: ExpressionContext.BlockHead }), + path + ) +); +EXPRESSIONS.add(SexpOpcodes.GetFreeInModifierHead, ([, sym, path]) => + withPath( + op('ResolveContextualFree', { freeVar: sym, context: ExpressionContext.ModifierHead }), + path + ) +); +EXPRESSIONS.add(SexpOpcodes.GetFreeInComponentHead, ([, sym, path]) => + withPath( + op('ResolveContextualFree', { freeVar: sym, context: ExpressionContext.ComponentHead }), + path + ) +); -EXPRESSIONS.add(SexpOpcodes.GetPath, ([, head, tail]) => { - return [op('Expr', head), ...tail.map(p => op(Op.GetProperty, p))]; -}); +function withPath(expr: ExpressionCompileActions, path?: string[]) { + if (path === undefined || path.length === 0) return expr; + if (!Array.isArray(expr)) expr = [expr]; -EXPRESSIONS.add(SexpOpcodes.GetFree, ([, head]) => op('ResolveFree', head)); + for (let i = 0; i < path.length; i++) { + expr.push(op(Op.GetProperty, path[i])); + } -EXPRESSIONS.add(SexpOpcodes.GetContextualFree, ([, head, context]) => - op('ResolveContextualFree', { freeVar: head, context }) -); + return expr; +} EXPRESSIONS.add(SexpOpcodes.Undefined, () => PushPrimitiveReference(undefined)); EXPRESSIONS.add(SexpOpcodes.HasBlock, ([, block]) => { diff --git a/packages/@glimmer/opcode-compiler/lib/syntax/macros.ts b/packages/@glimmer/opcode-compiler/lib/syntax/macros.ts index c2d0cbf90b..f068d4b572 100644 --- a/packages/@glimmer/opcode-compiler/lib/syntax/macros.ts +++ b/packages/@glimmer/opcode-compiler/lib/syntax/macros.ts @@ -15,7 +15,7 @@ import { } from '@glimmer/interfaces'; import { dict, assert } from '@glimmer/util'; import { UNHANDLED } from './concat'; -import { expectString } from '../utils'; +import { expectString, isGet, simplePathName } from '../utils'; export class MacrosImpl implements Macros { public blocks: MacroBlocks; @@ -115,7 +115,7 @@ export class Inlines implements MacroInlines { sexp: AppendSyntax, context: TemplateCompilationContext ): StatementCompileActions | Unhandled { - let [, , , , value] = sexp; + let [, , value] = sexp; // TODO: Fix this so that expression macros can return // things like components, so that {{component foo}} @@ -129,7 +129,7 @@ export class Inlines implements MacroInlines { if (value[0] === SexpOpcodes.Call) { let nameOrError = expectString( - value[3], + value[1], context.meta, 'Expected head of call to be a string' ); @@ -139,9 +139,9 @@ export class Inlines implements MacroInlines { } name = nameOrError; - params = value[4]; - hash = value[5]; - } else if (value[0] === SexpOpcodes.GetPath) { + params = value[2]; + hash = value[3]; + } else if (isGet(value)) { let pathName = simplePathName(value, context.meta); if (pathName === null) { @@ -173,18 +173,3 @@ export class Inlines implements MacroInlines { } } } - -function simplePathName( - [, get, tail]: WireFormat.Expressions.GetPath, - meta: ContainingMetadata -): Option { - if (tail.length > 0) { - return null; - } - - if (get[0] === SexpOpcodes.GetFree || get[0] === SexpOpcodes.GetContextualFree) { - return meta.upvars![get[1]]; - } - - return null; -} diff --git a/packages/@glimmer/opcode-compiler/lib/syntax/statements.ts b/packages/@glimmer/opcode-compiler/lib/syntax/statements.ts index a15925f2d7..bc7a254bcd 100644 --- a/packages/@glimmer/opcode-compiler/lib/syntax/statements.ts +++ b/packages/@glimmer/opcode-compiler/lib/syntax/statements.ts @@ -24,7 +24,7 @@ STATEMENTS.add(SexpOpcodes.CloseElement, () => op(Op.CloseElement)); STATEMENTS.add(SexpOpcodes.FlushElement, () => op(Op.FlushElement)); STATEMENTS.add(SexpOpcodes.Modifier, (sexp, meta) => { - let [, , , name, params, hash] = sexp; + let [, name, params, hash] = sexp; let stringName = expectString(name, meta, 'Expected modifier head to be a string'); @@ -162,7 +162,7 @@ STATEMENTS.add(SexpOpcodes.Debugger, ([, evalInfo], meta) => ); STATEMENTS.add(SexpOpcodes.Append, sexp => { - let [, trusted, , , value] = sexp; + let [, trusted, value] = sexp; if (typeof value === 'string' && trusted) { return op(Op.Text, value); diff --git a/packages/@glimmer/opcode-compiler/lib/utils.ts b/packages/@glimmer/opcode-compiler/lib/utils.ts index 21f6bd8267..204f7c8776 100644 --- a/packages/@glimmer/opcode-compiler/lib/utils.ts +++ b/packages/@glimmer/opcode-compiler/lib/utils.ts @@ -5,6 +5,8 @@ import { WireFormat, ContainingMetadata, CompileErrorOp, + Expressions, + SexpOpcodes, } from '@glimmer/interfaces'; import { dict, assign } from '@glimmer/util'; import { compilableBlock } from './compilable-template'; @@ -74,22 +76,41 @@ export function expectString( return error(`${desc}, but there were no free variables in the template`, 0, 0); } - if (!Array.isArray(expr) || expr[0] !== WireFormat.SexpOpcodes.GetPath) { + if (!Array.isArray(expr)) { throw new Error(`${desc}, got ${JSON.stringify(expr)}`); } - if (expr[2].length !== 0) { - throw new Error(`${desc}, got ${JSON.stringify(expr)}`); + if (isGet(expr)) { + let name = simplePathName(expr, meta); + if (name !== null) return name; } - if ( - expr[1][0] === WireFormat.SexpOpcodes.GetContextualFree || - expr[1][0] === WireFormat.SexpOpcodes.GetFree - ) { - let head = expr[1][1]; + throw new Error(`${desc}, got ${JSON.stringify(expr)}`); +} - return meta.upvars[head]; +export function simplePathName( + opcode: Expressions.GetPath | Expressions.Get, + meta: ContainingMetadata +): Option { + if (opcode.length === 3 && opcode[2].length > 0) { + return null; } - throw new Error(`${desc}, got ${JSON.stringify(expr)}`); + if (isGetFree(opcode)) { + return meta.upvars![opcode[1]]; + } + + return null; +} + +export function isGet( + opcode: Expressions.TupleExpression +): opcode is Expressions.Get | Expressions.GetPath { + return opcode.length >= 2 && opcode[0] >= SexpOpcodes.GetSymbol; +} + +function isGetFree( + opcode: Expressions.Get | Expressions.GetPath +): opcode is Expressions.GetFree | Expressions.GetContextualFree { + return opcode[0] >= SexpOpcodes.GetFree; } From 356976df56a41f8425d6f629740c3a3c4a87d2aa Mon Sep 17 00:00:00 2001 From: Kris Selden Date: Wed, 11 Mar 2020 13:26:52 -0700 Subject: [PATCH 2/2] Fix tests --- packages/@glimmer/compiler/lib/builder.ts | 2 +- packages/@glimmer/compiler/lib/wire-format-debug.ts | 8 ++++---- .../integration-tests/lib/suites/custom-dom-helper.ts | 4 ++-- packages/@glimmer/integration-tests/test/updating-test.ts | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@glimmer/compiler/lib/builder.ts b/packages/@glimmer/compiler/lib/builder.ts index 15a4c09093..fb83076f97 100644 --- a/packages/@glimmer/compiler/lib/builder.ts +++ b/packages/@glimmer/compiler/lib/builder.ts @@ -514,7 +514,7 @@ export function buildVar( op = Op.GetSymbol; sym = getSymbolForVar(head.kind, symbols, head.name); } - return (path === undefined ? [op, sym] : [op, sym, path]) as + return (path === undefined || path.length === 0 ? [op, sym] : [op, sym, path]) as | Expressions.Get | Expressions.GetPath; } diff --git a/packages/@glimmer/compiler/lib/wire-format-debug.ts b/packages/@glimmer/compiler/lib/wire-format-debug.ts index 79af940dff..3ce76ad47e 100644 --- a/packages/@glimmer/compiler/lib/wire-format-debug.ts +++ b/packages/@glimmer/compiler/lib/wire-format-debug.ts @@ -24,7 +24,7 @@ export default class WireFormatDebugger { if (Array.isArray(opcode)) { switch (opcode[0]) { case Op.Append: - return ['append', this.formatOpcode(opcode[1]), opcode[2]]; + return ['append', this.formatOpcode(opcode[1]), this.formatOpcode(opcode[2])]; case Op.Block: return [ @@ -94,7 +94,7 @@ export default class WireFormatDebugger { case Op.Component: return [ 'component', - opcode[1], + this.formatOpcode(opcode[1]), this.formatAttrs(opcode[2]), this.formatHash(opcode[3]), this.formatBlocks(opcode[4]), @@ -110,10 +110,10 @@ export default class WireFormatDebugger { // ]; case Op.HasBlock: - return ['has-block', opcode[1]]; + return ['has-block', this.formatOpcode(opcode[1])]; case Op.HasBlockParams: - return ['has-block-params', opcode[1]]; + return ['has-block-params', this.formatOpcode(opcode[1])]; case Op.Undefined: return ['undefined']; diff --git a/packages/@glimmer/integration-tests/lib/suites/custom-dom-helper.ts b/packages/@glimmer/integration-tests/lib/suites/custom-dom-helper.ts index 6ac8a11995..5e6864adaf 100644 --- a/packages/@glimmer/integration-tests/lib/suites/custom-dom-helper.ts +++ b/packages/@glimmer/integration-tests/lib/suites/custom-dom-helper.ts @@ -28,10 +28,10 @@ export class CompilationTests extends RenderTest { 'generates id in node'() { let template = precompile('hello'); let obj = JSON.parse(template); - this.assert.equal(obj.id, '14+n+4pQ', 'short sha of template source'); + this.assert.equal(obj.id, 'jbRFW0XF', 'short sha of template source'); template = precompile('hello', { meta: { moduleName: 'template/hello' } }); obj = JSON.parse(template); - this.assert.equal(obj.id, 'Ohs2KKrA', 'short sha of template source and meta'); + this.assert.equal(obj.id, 'djhwYnFp', 'short sha of template source and meta'); } } diff --git a/packages/@glimmer/integration-tests/test/updating-test.ts b/packages/@glimmer/integration-tests/test/updating-test.ts index 18ad12d7f1..9e07fd2d6b 100644 --- a/packages/@glimmer/integration-tests/test/updating-test.ts +++ b/packages/@glimmer/integration-tests/test/updating-test.ts @@ -775,7 +775,7 @@ class UpdatingTest extends RenderTest { assertHandleError(assert, result, { problem: 'Unexpected Helper helo', - span: { start: 2, end: 6 }, + span: { start: 0, end: 0 }, }); }