aot
/
compiler
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(preprocessor): finalize method override flags before conversion (#42)

Browse Source 
The classMethodOverride 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: the upward marking cannot cross the not-yet-registered
intermediate class, and the downward subclass lookup ran before the leaf
was prepared. findNativeMethod() then devirtualizes the late-bound call
into a direct native call, silently ignoring the override. In Hyperf this
turns SoftDeletes::delete() into a physical DELETE.

Finalize the flags once the complete class graph is known:
finalizeMethodOverrideFlags() walks every declared method's complete
parent chain and marks each existing ancestor method of the same name as
overridden (method count x inheritance depth). It runs from
Translator::convertFile() next to finalizeDeclarationExpressions(), so
both the project pipeline and the public prepareFile()/convertFile() API
share the same pre-conversion finalization, guarded by a dirty/finalized
flag reset in prepareFile().

Tests:
- tests/compiler/devirtualize/override-order-sandwich.phpt (fails on
  master with "base", passes with "leaf")
- tests/compiler/devirtualize/override-order-normal.phpt (control)
- phpunit/src/DevirtualizeOrderTest.php driving the public
  prepareFile()/convertFile() API in both orders, asserting the
  generated Base::delete() body dispatches dynamically
master
xiaoyin199 4 days ago committed by GitHub
parent d66f3d7e10
commit 8c0d52d330
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 209 additions and 0 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 16
      phpunit/code/devirtualize-order/base.php
  2. 11
      phpunit/code/devirtualize-order/leaf.php
  3. 7
      phpunit/code/devirtualize-order/mid.php
  4. 65
      phpunit/src/DevirtualizeOrderTest.php
  5. 1
      src/CompilerBase.php
  6. 43
      src/Preprocessor.php
  7. 1
      src/Translator.php
  8. 31
      tests/compiler/devirtualize/override-order-normal.phpt
  9. 34
      tests/compiler/devirtualize/override-order-sandwich.phpt

16
phpunit/code/devirtualize-order/base.php
Unescape View File

@ -0,0 +1,16 @@
<?php
namespace OrderTest;
class Base
{
protected function perform(): string
{
return 'base';
}
public function delete(): string
{
return $this->perform();
}
}

11
phpunit/code/devirtualize-order/leaf.php
Unescape View File

@ -0,0 +1,11 @@
<?php
namespace OrderTest;
class Leaf extends Mid
{
protected function perform(): string
{
return 'leaf';
}
}

7
phpunit/code/devirtualize-order/mid.php
Unescape View File

@ -0,0 +1,7 @@
<?php
namespace OrderTest;
class Mid extends Base
{
}

65
phpunit/src/DevirtualizeOrderTest.php
Unescape View File

@ -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
src/CompilerBase.php
Unescape View File

@ -333,6 +333,7 @@ class CompilerBase implements PropertyAccessContext
/** @var array<string, array<Node\Stmt>> Prepared declaration ASTs keyed by real path. */ /** @var array<string, array<Node\Stmt>> Prepared declaration ASTs keyed by real path. */
protected array $preparedFileAsts = []; protected array $preparedFileAsts = [];
protected bool $declarationExpressionsFinalized = false; protected bool $declarationExpressionsFinalized = false;
protected bool $methodOverrideFlagsFinalized = false;
protected const array PHP_RUNTIME_TYPE_MAP = [ protected const array PHP_RUNTIME_TYPE_MAP = [
'integer' => Type::INT, 'integer' => Type::INT,
'double' => Type::FLOAT, 'double' => Type::FLOAT,

43
src/Preprocessor.php
Unescape View File

@ -317,6 +317,9 @@ class Preprocessor extends CompilerBase
// generated until the complete symbol table is available. // generated until the complete symbol table is available.
$this->preparedFileAsts[$this->file] = $stmts; $this->preparedFileAsts[$this->file] = $stmts;
$this->declarationExpressionsFinalized = false; $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() // CompilerTest and embedding users may invoke prepareFile()
// directly instead of the project pipeline. Preserve same-file // directly instead of the project pipeline. Preserve same-file
// forward Native references for that public entry path as well. // forward Native references for that public entry path as well.
@ -2164,6 +2167,46 @@ class Preprocessor extends CompilerBase
$this->resetMethod(); $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( private function assertKeywordMethodMayBeDeclared(
Node\Stmt\ClassMethod $method, Node\Stmt\ClassMethod $method,
string $name, string $name,

1
src/Translator.php
Unescape View File

@ -548,6 +548,7 @@ class Translator extends Preprocessor
if (!$this->declarationExpressionsFinalized) { if (!$this->declarationExpressionsFinalized) {
$this->finalizeDeclarationExpressions(array_keys($this->preparedFileAsts)); $this->finalizeDeclarationExpressions(array_keys($this->preparedFileAsts));
} }
$this->finalizeMethodOverrideFlags();
$file = realpath($file); $file = realpath($file);
$phpCode = $this->loadFile($file); $phpCode = $this->loadFile($file);
$this->localHeaders = []; $this->localHeaders = [];

31
tests/compiler/devirtualize/override-order-normal.phpt
Unescape View File

@ -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
tests/compiler/devirtualize/override-order-sandwich.phpt
Unescape View File

@ -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"
Write Preview
Loading…
Cancel
Save