Skip to content

Commit 95986ee

Browse files
chancancodechancancode
authored
Merge pull request #337 from tildeio/fix-assert
Fix timing of expression evaluation in blocks
2 parents 6fa6478 + 1ed17a1 commit 95986ee

File tree

12 files changed

+233
-78
lines changed

12 files changed

+233
-78
lines changed

‎packages/glimmer-runtime/lib/compiled/opcodes/builder.ts

+18-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as component from './component';
2+
import * as partial from '../../compiled/opcodes/partial';
23
import * as content from './content';
34
import * as dom from './dom';
45
import * as lists from './lists';
@@ -11,6 +12,7 @@ import { Opcode, OpSeq } from '../../opcodes';
1112
import { CompiledArgs } from '../expressions/args';
1213
import { CompiledExpression } from '../expressions';
1314
import { ComponentDefinition } from '../../component/interfaces';
15+
import { PartialDefinition } from '../../partial';
1416
import Environment from '../../environment';
1517
import { InlineBlock, Layout } from '../blocks';
1618
import { EMPTY_ARRAY } from '../../utils';
@@ -117,6 +119,20 @@ export abstract class BasicOpcodeBuilder extends StatementCompilationBufferProxy
117119
return label;
118120
}
119121

122+
// partials
123+
124+
putPartialDefinition(definition: PartialDefinition<Opaque>) {
125+
this.append(new partial.PutPartialDefinitionOpcode(definition));
126+
}
127+
128+
putDynamicPartialDefinition() {
129+
this.append(new partial.PutDynamicPartialDefinitionOpcode(this.symbolTable));
130+
}
131+
132+
evaluatePartial() {
133+
this.append(new partial.EvaluatePartialOpcode(this.symbolTable));
134+
}
135+
120136
// components
121137

122138
putComponentDefinition(definition: ComponentDefinition<Opaque>) {
@@ -362,13 +378,13 @@ export default class OpcodeBuilder extends BasicOpcodeBuilder {
362378
// TODO
363379
// come back to this
364380
block({ templates, args }: BlockArgs, callback: BlockCallback) {
381+
if (args) this.putArgs(args);
382+
365383
this.startLabels();
366384
this.startBlock(templates);
367385
this.enter('BEGIN', 'END');
368386
this.label('BEGIN');
369387

370-
if (args) this.putArgs(args);
371-
372388
callback(this, 'BEGIN', 'END');
373389

374390
this.label('END');

‎packages/glimmer-runtime/lib/compiled/opcodes/content.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,11 @@ export abstract class GuardedAppendOpcode<T extends Insertion> extends AppendOpc
156156
// expanded version that handles both cases. The compilation would look
157157
// like this:
158158
//
159+
// PutValue(expression)
160+
// Test(is-component-definition)
159161
// Enter(BEGIN, END)
160162
// BEGIN: Noop
161-
// Test(is-component-definition)
162163
// JumpUnless(VALUE)
163-
// COMPONENT: Noop
164164
// PutDynamicComponentDefinitionOpcode
165165
// OpenComponent
166166
// CloseComponent
@@ -179,11 +179,11 @@ export abstract class GuardedAppendOpcode<T extends Insertion> extends AppendOpc
179179
let buffer = new CompileIntoList(env, null);
180180
let dsl = new OpcodeBuilderDSL(buffer, this.symbolTable, env);
181181

182-
dsl.block({ templates: null }, (dsl, BEGIN, END) => {
183-
dsl.putValue(this.expression);
184-
dsl.test(IsComponentDefinitionReference.create);
182+
dsl.putValue(this.expression);
183+
dsl.test(IsComponentDefinitionReference.create);
184+
185+
dsl.simpleBlock((dsl, BEGIN, END) => {
185186
dsl.jumpUnless('VALUE');
186-
dsl.label('COMPONENT');
187187
dsl.putDynamicComponentDefinition();
188188
dsl.openComponent(Args.empty());
189189
dsl.closeComponent();
@@ -335,14 +335,16 @@ abstract class GuardedUpdateOpcode<T extends Insertion> extends UpdateOpcode<T>
335335
let { bounds, appendOpcode, state } = this;
336336

337337
let appendOps = appendOpcode.deopt(vm.env);
338-
let enter = appendOps.head() as EnterOpcode;
338+
let enter = appendOps.head().next.next as EnterOpcode;
339339
let ops = enter.slice;
340340

341341
let tracker = new UpdatableBlockTracker(bounds.parentElement());
342342
tracker.newBounds(this.bounds);
343343

344344
let children = new LinkedList<UpdatingOpcode>();
345345

346+
state.frame['condition'] = IsComponentDefinitionReference.create(state.frame['operand']);
347+
346348
let deopted = this.deopted = new TryOpcode(ops, state, tracker, children);
347349

348350
this._tag.update(deopted.tag);

‎packages/glimmer-runtime/lib/compiler.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,11 @@ class ComponentBuilder implements IComponentBuilder {
399399

400400
dynamic(definitionArgs: Syntax.Args, definition: DynamicDefinition, args: Syntax.Args, templates: Syntax.Templates, symbolTable: SymbolTable, shadow: string[] = EMPTY_ARRAY) {
401401
this.dsl.unit({ templates }, dsl => {
402-
dsl.enter('BEGIN', 'END');
403-
dsl.label('BEGIN');
404402
dsl.putArgs(definitionArgs);
405403
dsl.putValue(makeFunctionExpression(definition));
406404
dsl.test('simple');
405+
dsl.enter('BEGIN', 'END');
406+
dsl.label('BEGIN');
407407
dsl.jumpUnless('END');
408408
dsl.putDynamicComponentDefinition();
409409
dsl.openComponent(args, shadow);

‎packages/glimmer-runtime/lib/syntax/builtins/if.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ export default class IfSyntax extends StatementSyntax {
2020
}
2121

2222
compile(dsl: OpcodeBuilderDSL) {
23-
// Enter(BEGIN, END)
24-
// BEGIN: Noop
2523
// PutArgs
2624
// Test(Environment)
25+
// Enter(BEGIN, END)
26+
// BEGIN: Noop
2727
// JumpUnless(ELSE)
2828
// Evaluate(default)
2929
// Jump(END)
@@ -34,9 +34,10 @@ export default class IfSyntax extends StatementSyntax {
3434

3535
let { args, templates } = this;
3636

37-
dsl.block({ templates, args }, (dsl, BEGIN, END) => {
38-
dsl.test('environment');
37+
dsl.putArgs(args);
38+
dsl.test('environment');
3939

40+
dsl.block({ templates }, (dsl, BEGIN, END) => {
4041
if (templates.inverse) {
4142
dsl.jumpUnless('ELSE');
4243
dsl.evaluate('default');

‎packages/glimmer-runtime/lib/syntax/builtins/in-element.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@ export default class InElementSyntax extends StatementSyntax {
2222
compile(dsl: OpcodeBuilderDSL, env: Environment) {
2323
let { args, templates } = this;
2424

25-
dsl.block({ templates, args }, (dsl, BEGIN, END) => {
26-
dsl.putArgs(args);
27-
dsl.test('simple');
25+
dsl.putArgs(args);
26+
dsl.test('simple');
27+
28+
dsl.block({ templates }, (dsl, BEGIN, END) => {
2829
dsl.jumpUnless(END);
2930
dsl.pushRemoteElement();
3031
dsl.evaluate('default');

‎packages/glimmer-runtime/lib/syntax/builtins/partial.ts

+20-47
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,13 @@
11
import { Opaque } from "glimmer-util";
22

33
import {
4-
CompileInto,
5-
SymbolLookup,
64
Statement as StatementSyntax,
75
Expression as ExpressionSyntax
86
} from '../../syntax';
97

108
import SymbolTable from '../../symbol-table';
119

12-
import {
13-
LabelOpcode,
14-
EnterOpcode,
15-
PutValueOpcode,
16-
SimpleTest,
17-
TestOpcode,
18-
JumpUnlessOpcode,
19-
ExitOpcode
20-
} from '../../compiled/opcodes/vm';
21-
22-
import {
23-
PutPartialDefinitionOpcode,
24-
PutDynamicPartialDefinitionOpcode,
25-
EvaluatePartialOpcode
26-
} from '../../compiled/opcodes/partial';
10+
import OpcodeBuilderDSL from '../../compiled/opcodes/builder';
2711

2812
import * as Syntax from '../core';
2913
import Environment from '../../environment';
@@ -35,7 +19,7 @@ export class StaticPartialSyntax extends StatementSyntax {
3519
super();
3620
}
3721

38-
compile(compiler: CompileInto & SymbolLookup, env: Environment, symbolTable: SymbolTable) {
22+
compile(dsl: OpcodeBuilderDSL, env: Environment, symbolTable: SymbolTable) {
3923
let name = String(this.name.inner());
4024

4125
if (!env.hasPartial(name, symbolTable)) {
@@ -44,8 +28,8 @@ export class StaticPartialSyntax extends StatementSyntax {
4428

4529
let definition = env.lookupPartial(name, symbolTable);
4630

47-
compiler.append(new PutPartialDefinitionOpcode(definition));
48-
compiler.append(new EvaluatePartialOpcode(symbolTable));
31+
dsl.putPartialDefinition(definition);
32+
dsl.evaluatePartial();
4933
}
5034
}
5135

@@ -56,32 +40,21 @@ export class DynamicPartialSyntax extends StatementSyntax {
5640
super();
5741
}
5842

59-
compile(compiler: CompileInto & SymbolLookup, env: Environment, symbolTable: SymbolTable) {
60-
let name = this.name.compile(compiler, env, symbolTable);
61-
62-
/*
63-
// Enter(BEGIN, END)
64-
// BEGIN: Noop
65-
// PutArgs
66-
// NameToPartial
67-
// Test
68-
// JumpUnless(END)
69-
// EvaluatePartial
70-
// END: Noop
71-
// Exit
72-
*/
73-
74-
let BEGIN = new LabelOpcode("BEGIN");
75-
let END = new LabelOpcode("END");
76-
77-
compiler.append(new EnterOpcode(BEGIN, END));
78-
compiler.append(BEGIN);
79-
compiler.append(new PutValueOpcode(name));
80-
compiler.append(new TestOpcode(SimpleTest));
81-
compiler.append(new JumpUnlessOpcode(END));
82-
compiler.append(new PutDynamicPartialDefinitionOpcode(symbolTable));
83-
compiler.append(new EvaluatePartialOpcode(symbolTable));
84-
compiler.append(END);
85-
compiler.append(new ExitOpcode());
43+
compile(dsl: OpcodeBuilderDSL) {
44+
let { name } = this;
45+
46+
dsl.startLabels();
47+
48+
dsl.putValue(name);
49+
dsl.test('simple');
50+
dsl.enter('BEGIN', 'END');
51+
dsl.label('BEGIN');
52+
dsl.jumpUnless('END');
53+
dsl.putDynamicPartialDefinition();
54+
dsl.evaluatePartial();
55+
dsl.label('END');
56+
dsl.exit();
57+
58+
dsl.stopLabels();
8659
}
8760
}

‎packages/glimmer-runtime/lib/syntax/builtins/unless.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ export default class UnlessSyntax extends StatementSyntax {
2222
}
2323

2424
compile(dsl: OpcodeBuilderDSL, env: Environment) {
25+
// PutArgs
2526
// Enter(BEGIN, END)
2627
// BEGIN: Noop
27-
// PutArgs
2828
// Test(Environment)
2929
// JumpIf(ELSE)
3030
// Evaluate(default)
@@ -36,9 +36,10 @@ export default class UnlessSyntax extends StatementSyntax {
3636

3737
let { args, templates } = this;
3838

39-
dsl.block({ templates, args }, dsl => {
40-
dsl.test('environment');
39+
dsl.putArgs(args);
40+
dsl.test('environment');
4141

42+
dsl.block({ templates }, dsl => {
4243
if (templates.inverse) {
4344
dsl.jumpIf('ELSE');
4445
dsl.evaluate('default');

‎packages/glimmer-runtime/lib/syntax/builtins/with.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ export default class WithSyntax extends StatementSyntax {
2121
}
2222

2323
compile(dsl: OpcodeBuilderDSL, env: Environment) {
24-
// Enter(BEGIN, END)
25-
// BEGIN: Noop
2624
// PutArgs
2725
// Test(Environment)
26+
// Enter(BEGIN, END)
27+
// BEGIN: Noop
2828
// JumpUnless(ELSE)
2929
// Evaluate(default)
3030
// Jump(END)
@@ -35,9 +35,10 @@ export default class WithSyntax extends StatementSyntax {
3535

3636
let { args, templates } = this;
3737

38-
dsl.block({ templates, args }, (dsl, BEGIN, END) => {
39-
dsl.test('environment');
38+
dsl.putArgs(args);
39+
dsl.test('environment');
4040

41+
dsl.block({ templates }, (dsl, BEGIN, END) => {
4142
if (templates.inverse) {
4243
dsl.jumpUnless('ELSE');
4344
dsl.evaluate('default');

‎packages/glimmer-runtime/lib/vm/append.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { Range } from '../utils';
1212
import { Component, ComponentManager } from '../component/interfaces';
1313
import { VMState, ListBlockOpcode, TryOpcode, BlockOpcode } from './update';
1414
import RenderResult from './render-result';
15-
import { FrameStack, Blocks } from './frame';
15+
import { CapturedFrame, FrameStack, Blocks } from './frame';
1616

1717
interface VMInitialOptions {
1818
self: PathReference<Opaque>;
@@ -73,7 +73,8 @@ export default class VM implements PublicVM {
7373
return {
7474
env: this.env,
7575
scope: this.scope(),
76-
dynamicScope: this.dynamicScope()
76+
dynamicScope: this.dynamicScope(),
77+
frame: this.frame.capture()
7778
};
7879
}
7980

@@ -263,6 +264,10 @@ export default class VM implements PublicVM {
263264

264265
/// EXECUTION
265266

267+
resume(opcodes: OpSeq, frame: CapturedFrame): RenderResult {
268+
return this.execute(opcodes, vm => vm.frame.restore(frame));
269+
}
270+
266271
execute(opcodes: OpSeq, initialize?: (vm: VM) => void): RenderResult {
267272
LOGGER.debug("[VM] Begin program execution");
268273

‎packages/glimmer-runtime/lib/vm/frame.ts

+26
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ import { Opcode, OpSeq } from '../opcodes';
66
import { LabelOpcode } from '../compiled/opcodes/vm';
77
import { Component, ComponentManager } from '../component/interfaces';
88

9+
export class CapturedFrame {
10+
constructor(
11+
private operand: PathReference<any>,
12+
private args: EvaluatedArgs,
13+
private condition: Reference<boolean>
14+
) {}
15+
}
16+
917
class Frame {
1018
ops: OpSeq;
1119
op: Opcode;
@@ -27,6 +35,16 @@ class Frame {
2735
this.ops = ops;
2836
this.op = ops.head();
2937
}
38+
39+
capture(): CapturedFrame {
40+
return new CapturedFrame(this.operand, this.args, this.condition);
41+
}
42+
43+
restore(frame: CapturedFrame) {
44+
this.operand = frame['operand'];
45+
this.args = frame['args'];
46+
this.condition = frame['condition'];
47+
}
3048
}
3149

3250
export interface Blocks {
@@ -54,6 +72,14 @@ export class FrameStack {
5472
this.frame = frame === 0 ? undefined : frame - 1;
5573
}
5674

75+
capture(): CapturedFrame {
76+
return this.frames[this.frame].capture();
77+
}
78+
79+
restore(frame: CapturedFrame) {
80+
this.frames[this.frame].restore(frame);
81+
}
82+
5783
getOps(): OpSeq {
5884
return this.frames[this.frame].ops;
5985
}

0 commit comments

Comments
 (0)