Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions phpunit/code/devirtualize-order/base.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace OrderTest;

class Base
{
protected function perform(): string
{
return 'base';
}

public function delete(): string
{
return $this->perform();
}
}
11 changes: 11 additions & 0 deletions phpunit/code/devirtualize-order/leaf.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace OrderTest;

class Leaf extends Mid
{
protected function perform(): string
{
return 'leaf';
}
}
7 changes: 7 additions & 0 deletions phpunit/code/devirtualize-order/mid.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace OrderTest;

class Mid extends Base
{
}
65 changes: 65 additions & 0 deletions phpunit/src/DevirtualizeOrderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php
/**
* This file is part of TypePHP(AOT).
*
* @link https://www.swoole.com/aot/
* @contact service@swoole.com
*/

namespace TypePhp\Tests;

use PHPUnit\Framework\TestCase;
use TypePhp\CompilerTest;

/**
* @internal
* @coversNothing
*/
class DevirtualizeOrderTest extends TestCase
{
public function testSandwichDeclarationOrderKeepsOverrideDynamic(): void
{
// Ancestor first, leaf second, intermediate class last: the ancestor
// method's override flag used to be missed, and the late-bound call
// in Base::delete() was wrongly devirtualized to a direct native call.
$cpp = $this->compileBaseInOrder(['base.php', 'leaf.php', 'mid.php']);
$this->assertDeleteDispatchesDynamically($cpp);
}

public function testNormalDeclarationOrderKeepsOverrideDynamic(): void
{
// Control: ancestor, intermediate, leaf.
$cpp = $this->compileBaseInOrder(['base.php', 'mid.php', 'leaf.php']);
$this->assertDeleteDispatchesDynamically($cpp);
}

/** @param list<string> $order */
private function compileBaseInOrder(array $order): string
{
global $translator;

$dir = TYPEPHP_ROOT_PATH . '/phpunit/code/devirtualize-order';
$compiler = CompilerTest::create(TYPEPHP_ROOT_PATH);
$translator = $compiler;
foreach ($order as $file) {
$compiler->addFiles([$dir . '/' . $file]);
$compiler->prepareFile($dir . '/' . $file);
}

return file_get_contents($compiler->convertFile($dir . '/base.php'));
}

private function assertDeleteDispatchesDynamically(string $cpp): void
{
$matched = preg_match(
'/php::\w+ php_ordertest__base__delete\(php::Object &this_\) \{(?<body>.*?)\n\}/s',
$cpp,
$m,
);
self::assertSame(1, $matched, 'generated body of OrderTest\\Base::delete() not found');
// A direct native call to the base implementation means the override
// was wrongly devirtualized; the call must go through dynamic dispatch.
self::assertStringNotContainsString('php_ordertest__base__perform', $m['body']);
self::assertStringContainsString('callScoped', $m['body']);
}
}
1 change: 1 addition & 0 deletions src/CompilerBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ protected function getBoolValue(Expr\ConstFetch $expr): string
/** @var array<string, array<Node\Stmt>> Prepared declaration ASTs keyed by real path. */
protected array $preparedFileAsts = [];
protected bool $declarationExpressionsFinalized = false;
protected bool $methodOverrideFlagsFinalized = false;
protected const array PHP_RUNTIME_TYPE_MAP = [
'integer' => Type::INT,
'double' => Type::FLOAT,
Expand Down
43 changes: 43 additions & 0 deletions src/Preprocessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ public function prepareFile(string $file): void
// generated until the complete symbol table is available.
$this->preparedFileAsts[$this->file] = $stmts;
$this->declarationExpressionsFinalized = false;
// The prepared class graph changed; override flags must be
// re-finalized before the next conversion.
$this->methodOverrideFlagsFinalized = false;
// CompilerTest and embedding users may invoke prepareFile()
// directly instead of the project pipeline. Preserve same-file
// forward Native references for that public entry path as well.
Expand Down Expand Up @@ -2164,6 +2167,46 @@ protected function prepareClassMethod(Node\Stmt\ClassMethod $v, Node\Stmt\Class_
$this->resetMethod();
}

/**
* Finalize the classMethodOverride flags once the complete class graph is
* known.
*
* The incremental registration in prepareClassMethod() depends on file
* preprocessing order: with a "sandwich" order (ancestor first, leaf
* second, intermediate class last), the ancestor method's override flag is
* missed, causing MethodCallTrait::findNativeMethod() to devirtualize a
* call that should be dynamically dispatched.
*
* Runs once per class-graph change, before conversion starts. For every
* declared method it walks the complete parent chain and marks each
* ancestor method of the same name as overridden, following the existing
* upward-marking semantics. This is order-independent and costs roughly
* method count x inheritance depth.
*/
protected function finalizeMethodOverrideFlags(): void
{
$this->assertCompilerPhase(self::PHASE_CONVERT, 'method override flag finalization');
if ($this->methodOverrideFlagsFinalized) {
return;
}
$this->methodOverrideFlagsFinalized = true;
foreach (array_keys($this->classMethodOverride) as $fullMethodNameLower) {
$pos = strrpos($fullMethodNameLower, '::');
if ($pos === false) {
continue;
}
$methodLower = substr($fullMethodNameLower, $pos + 2);
$classLower = substr($fullMethodNameLower, 0, $pos);
while (($parentClass = $this->symbols->parent($classLower)) !== '') {
$parentMethodLower = strtolower($parentClass) . '::' . $methodLower;
if (isset($this->classMethodOverride[$parentMethodLower])) {
$this->classMethodOverride[$parentMethodLower] = true;
}
$classLower = strtolower($parentClass);
}
}
}

private function assertKeywordMethodMayBeDeclared(
Node\Stmt\ClassMethod $method,
string $name,
Expand Down
1 change: 1 addition & 0 deletions src/Translator.php
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ public function convertFile(string $file): ?string
if (!$this->declarationExpressionsFinalized) {
$this->finalizeDeclarationExpressions(array_keys($this->preparedFileAsts));
}
$this->finalizeMethodOverrideFlags();
$file = realpath($file);
$phpCode = $this->loadFile($file);
$this->localHeaders = [];
Expand Down
31 changes: 31 additions & 0 deletions tests/compiler/devirtualize/override-order-normal.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
Devirtualize: override flag with normal declaration order (ancestor, intermediate, leaf) - control
--FILE--
<?php

class OrderedBase {
protected function perform(): string {
return "base";
}

public function delete(): string {
return $this->perform();
}
}

class OrderedMid extends OrderedBase {
}

class OrderedLeaf extends OrderedMid {
protected function perform(): string {
return "leaf";
}
}

function main() {
var_dump((new OrderedLeaf())->delete());
}

?>
--EXPECT--
string(4) "leaf"
34 changes: 34 additions & 0 deletions tests/compiler/devirtualize/override-order-sandwich.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
Devirtualize: override flag survives sandwich declaration order (ancestor, leaf, intermediate)
--FILE--
<?php

class SandwichBase {
protected function perform(): string {
return "base";
}

public function delete(): string {
return $this->perform();
}
}

// Leaf is declared before its parent SandwichMid: the override flag of
// SandwichBase::perform() must still be registered, so the late-bound call
// in delete() stays dynamic.
class SandwichLeaf extends SandwichMid {
protected function perform(): string {
return "leaf";
}
}

class SandwichMid extends SandwichBase {
}

function main() {
var_dump((new SandwichLeaf())->delete());
}

?>
--EXPECT--
string(4) "leaf"