Skip to content

Commit cd13cdc

Browse files
committed
Test hoisting block program closures
Segmentation fault on 12+ iterations of benchmark.
1 parent 397cb3d commit cd13cdc

2 files changed

Lines changed: 77 additions & 7 deletions

File tree

src/Compiler.php

Lines changed: 76 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ final class Compiler
4545
*/
4646
private array $blockParamValues = [];
4747

48+
/**
49+
* Hoisted block program closures: varName => closure string.
50+
* Populated during main-template compilation; emitted before the render closure in composePHPRender
51+
* so they are created once per template() call and reused across all render calls.
52+
* Only populated when $isHoistingEnabled is true (main compiler instance, not partial compilers).
53+
* @var array<string, string>
54+
*/
55+
private array $hoistedPrograms = [];
56+
private int $hoistedProgramIdx = 0;
57+
4858
/**
4959
* Stack of booleans, one per active compileProgram() call.
5060
* Each entry is set to true if that invocation directly emitted a $blockParams reference.
@@ -66,8 +76,13 @@ final class Compiler
6676
*/
6777
private bool $compilingHelperArgs = false;
6878

79+
/**
80+
* @param bool $isHoistingEnabled Only true for the main-template Compiler. Partial compilers use the default
81+
* so their block programs remain inline (partial closures are recreated per render call anyway).
82+
*/
6983
public function __construct(
7084
private readonly Parser $parser,
85+
private readonly bool $isHoistingEnabled = false,
7186
) {}
7287

7388
public function compile(Program $program, Context $context): string
@@ -76,6 +91,8 @@ public function compile(Program $program, Context $context): string
7691
$this->blockParamValues = [];
7792
$this->bpRefStack = [];
7893
$this->lastCompileProgramHadDirectBpRef = false;
94+
$this->hoistedPrograms = [];
95+
$this->hoistedProgramIdx = 0;
7996
return $this->compileProgram($program);
8097
}
8198

@@ -87,8 +104,15 @@ public function compile(Program $program, Context $context): string
87104
public function composePHPRender(string $code): string
88105
{
89106
$partials = implode(",\n", $this->context->partialCode);
90-
$closure = self::templateClosure($code, $partials, "\n \$in = &\$cx->data['root'];");
91-
return "use " . Runtime::class . " as LR;\nreturn $closure;";
107+
$useVars = $this->hoistedPrograms ? implode(', ', array_keys($this->hoistedPrograms)) : '';
108+
$closure = self::templateClosure($code, $partials, "\n \$in = &\$cx->data['root'];", $useVars);
109+
110+
$assignments = '';
111+
foreach ($this->hoistedPrograms as $var => $closureStr) {
112+
$assignments .= "$var = $closureStr;\n";
113+
}
114+
115+
return "use " . Runtime::class . " as LR;\n{$assignments}return $closure;";
92116
}
93117

94118
/**
@@ -234,8 +258,14 @@ private function blockParamsUseVars(): string
234258

235259
/**
236260
* Compile a block program, pushing/popping its block params around the compilation.
237-
* Returns a PHP closure string: the signature varies based on whether the program declares or
238-
* inherits block params, and a $sc preamble is added when depths are accessed multiple times.
261+
* Returns either a hoisted variable reference (e.g. '$p0') or an inline closure string.
262+
*
263+
* Closures that do not capture any runtime variable ($declaresBp receive $blockParams as a
264+
* parameter; default programs have no block-param dependency) are hoisted to eval-scope
265+
* variables so they are created once at template() time and reused across render calls.
266+
*
267+
* The one non-hoistable case is $inheritsBp && !$declaresBp: the closure must capture
268+
* $blockParams from its enclosing runtime scope via use(), so it must stay inline.
239269
*/
240270
private function compileProgramWithBlockParams(Program $program): string
241271
{
@@ -260,6 +290,26 @@ private function compileProgramWithBlockParams(Program $program): string
260290
$inheritsBp => "function(\$cx, \$in) use (\$blockParams)",
261291
default => "function(\$cx, \$in)",
262292
};
293+
// Hoist closures that capture no runtime variable. The use($blockParams) case
294+
// ($inheritsBp && !$declaresBp) must remain inline; all others are safe to hoist.
295+
if ($this->isHoistingEnabled && !($inheritsBp && !$declaresBp)) {
296+
$varName = '$p' . $this->hoistedProgramIdx++;
297+
// Inner programs compile before outer ones (depth-first), so any $pN referenced in
298+
// this body was already assigned a lower index and will be emitted first. Add a
299+
// use() clause so the hoisted closure can access those sibling variables.
300+
preg_match_all('/\$p\d+/', $preamble . $body, $matches);
301+
$refs = array_unique($matches[0]);
302+
$useClause = $refs ? ' use (' . implode(', ', $refs) . ')' : '';
303+
$this->hoistedPrograms[$varName] = "$sig$useClause {{$preamble}return $body;}";
304+
return $varName;
305+
}
306+
307+
if ($inheritsBp && !$declaresBp) {
308+
// Non-hoistable: use($blockParams) captures a runtime value, so the closure must stay
309+
// inline. Still extend its use() clause with any hoisted $pN it references.
310+
$extendedUse = $this->extendUseVarsWithHoistedRefs('$blockParams', $preamble . $body);
311+
return "function(\$cx, \$in) use ($extendedUse) {{$preamble}return $body;}";
312+
}
263313
return "$sig {{$preamble}return $body;}";
264314
}
265315

@@ -373,7 +423,8 @@ private function DecoratorBlock(BlockStatement $block): string
373423
$this->context->usedPartial[$partialName] = '';
374424

375425
// Capture $blockParams if we're inside a block-param scope so the inline partial body can access them.
376-
$useVars = $this->blockParamsUseVars();
426+
// Also capture any hoisted $pN programs referenced in the body.
427+
$useVars = $this->extendUseVarsWithHoistedRefs($this->blockParamsUseVars(), $body);
377428
$escapedName = self::quote($partialName);
378429
return self::getRuntimeFunc('in', "\$cx, $escapedName, " . self::templateClosure($body, useVars: $useVars));
379430
}
@@ -451,7 +502,8 @@ private function PartialBlockStatement(PartialBlockStatement $statement): string
451502
$vars = $this->compilePartialParams($statement->params, $statement->hash);
452503

453504
// Capture $blockParams if we're inside a block-param scope so the partial block body can access them.
454-
$useVars = $this->blockParamsUseVars();
505+
// Also capture any hoisted $pN programs referenced in the body.
506+
$useVars = $this->extendUseVarsWithHoistedRefs($this->blockParamsUseVars(), $body);
455507
$bodyClosure = self::templateClosure($body, useVars: $useVars);
456508

457509
if ($partialName !== null && !$found) {
@@ -930,4 +982,22 @@ private function throwKnownHelpersOnly(string $helperName): never
930982
{
931983
throw new \Exception("You specified knownHelpersOnly, but used the unknown helper $helperName");
932984
}
985+
986+
/**
987+
* Extends $useVars with any $pN hoisted program references found in $code.
988+
* Called before templateClosure() and for the non-hoistable use($blockParams) closure case,
989+
* so that all closures can see the hoisted program variables they reference.
990+
*/
991+
private function extendUseVarsWithHoistedRefs(string $useVars, string $code): string
992+
{
993+
if (!$this->hoistedPrograms) {
994+
return $useVars;
995+
}
996+
preg_match_all('/\$p\d+/', $code, $matches);
997+
$refs = array_unique($matches[0]);
998+
if (!$refs) {
999+
return $useVars;
1000+
}
1001+
return $useVars ? $useVars . ', ' . implode(', ', $refs) : implode(', ', $refs);
1002+
}
9331003
}

src/Handlebars.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public static function compile(string $template, Options $options = new Options(
3030
public static function precompile(string $template, Options $options = new Options()): string
3131
{
3232
self::$parser ??= (new ParserFactory())->create();
33-
self::$compiler ??= new Compiler(self::$parser);
33+
self::$compiler ??= new Compiler(self::$parser, isHoistingEnabled: true);
3434

3535
$context = new Context($options);
3636
$program = self::$parser->parse($template, $options->ignoreStandalone);

0 commit comments

Comments
 (0)