From e9ce4dcf1d1c53aca34256deb74a50a0aa89a6ce Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 27 Jul 2026 17:56:55 +0200 Subject: [PATCH 1/3] Repeat a shadowed class's interfaces on its stub shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated stub only extended the native class, so every interface the shadowed class declared disappeared with the extension active: PhpParser\NodeTraverser stopped being an instanceof NodeTraverserInterface, and a shadowed service would silently lose the tags PHPStan's own autowiring derives from implementsInterface(). The shell now repeats the implements clause and requires the interface sources itself, because it is declared before the Composer autoloader registers. The engine then enforces variance against those interfaces, so reg::Class can declare a method's return type — NodeTraverser's three interface methods needed it. A parent class cannot be preserved the same way, PHP being single-inheritance, so the collector now rejects a shadowed class that has one instead of dropping it silently. The DI-service rule in the README described the class_alias approach the C++ rewrite replaced: getByType() normalizes through reflection to the real class name, which a stub subclass carrying the original name is. What a shadowed service does require is a constructor arginfo declaring the real parameter class names, since Nette autowires by reflecting it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016XhKccs7xeB1wRTAQEemc2 --- .../PHPStan/Build/TurboAttributeCollector.php | 84 +++++++++++++++++-- build/generate-turbo-stubs.php | 3 +- src/Turbo/ShadowedByTurboExtension.php | 4 +- turbo-ext/CLAUDE.md | 9 +- turbo-ext/README.md | 16 +++- turbo-ext/src/NodeTraverser.cpp | 9 +- turbo-ext/src/reg.h | 10 ++- 7 files changed, 115 insertions(+), 20 deletions(-) diff --git a/build/PHPStan/Build/TurboAttributeCollector.php b/build/PHPStan/Build/TurboAttributeCollector.php index fa0d80001fb..197a805ab7c 100644 --- a/build/PHPStan/Build/TurboAttributeCollector.php +++ b/build/PHPStan/Build/TurboAttributeCollector.php @@ -35,6 +35,7 @@ use ReflectionClass; use RuntimeException; use Throwable; +use function array_keys; use function class_exists; use function count; use function file_get_contents; @@ -143,7 +144,7 @@ public function __construct(string $root) * the path) and merges the hardcoded vendored entries. * * @return array{ - * pairs: array, + * pairs: array}>, * manifest: array, * classMap: array, * referenced: array, @@ -195,6 +196,28 @@ public function collect(): array } } + foreach (array_keys($pairs) as $className) { + $reflection = new ReflectionClass($className); + + // The stub shell extends the native class, and PHP is + // single-inheritance, so a parent of the shadowed class would + // silently disappear together with everything it declares. + $parent = $reflection->getParentClass(); + if ($parent !== false) { + throw new RuntimeException(sprintf( + '%s cannot be shadowed by the turbo extension because it extends %s — the stub shell already extends the native class.', + $className, + $parent->getName(), + )); + } + + // Interfaces do survive, but only because they are re-declared on + // the stub: it inherits from the native class alone, so an + // interface left out here silently stops matching instanceof and + // DI type lookups. + $pairs[$className][3] = $reflection->getInterfaceNames(); + } + ksort($pairs); ksort($classMap); ksort($referenced); @@ -219,24 +242,74 @@ public function collect(): array return ['pairs' => $pairs, 'manifest' => $manifest, 'classMap' => $classMap, 'referenced' => $referenced]; } - /** @param array $pairs */ + /** + * Parent interfaces first, deduplicated, keyed by name to keep the order + * stable across calls. + * + * @param class-string $interface + * @param array $files + */ + private static function collectInterfaceFiles(string $interface, array &$files): void + { + if (isset($files[$interface])) { + return; + } + + $reflection = new ReflectionClass($interface); + foreach ($reflection->getInterfaceNames() as $parent) { + self::collectInterfaceFiles($parent, $files); + } + + $fileName = $reflection->getFileName(); + if ($fileName === false) { + return; // an engine interface, always declared + } + + $files[$interface] = $fileName; + } + + /** @param array}> $pairs */ public function renderStubs(array $pairs): string { $namespaces = []; - foreach ($pairs as $className => [$turboClass, $final]) { + foreach ($pairs as $className => [$turboClass, $final, $cppFile, $interfaces]) { $pos = strrpos($className, '\\'); if ($pos === false) { throw new RuntimeException(sprintf('%s is not a namespaced class name', $className)); } + $implements = ''; + if (count($interfaces) > 0) { + $implements = ' implements \\' . implode(', \\', $interfaces); + } $namespaces[substr($className, 0, $pos)][] = sprintf( - "\t%sclass %s extends \\%s {}", + "\t%sclass %s extends \\%s%s {}", $final ? 'final ' : '', substr($className, $pos + 1), $turboClass, + $implements, ); } $blocks = []; + + // The stubs are declared before the Composer autoloader registers, so + // the interfaces they re-declare have to be required here — parents + // first, since an interface extending another needs it at declaration + // time. Interfaces without a file are the engine's own. + $interfaceFiles = []; + foreach ($pairs as $pair) { + foreach ($pair[3] as $interface) { + self::collectInterfaceFiles($interface, $interfaceFiles); + } + } + if (count($interfaceFiles) > 0) { + $requires = []; + foreach ($interfaceFiles as $file) { + $requires[] = sprintf("\trequire_once __DIR__ . '/../%s';", $this->relativize($file)); + } + $blocks[] = sprintf("namespace {\n\n%s\n\n}", implode("\n", $requires)); + } + foreach ($namespaces as $namespace => $declarations) { $blocks[] = sprintf("namespace %s {\n\n%s\n\n}", $namespace, implode("\n", $declarations)); } @@ -250,7 +323,8 @@ public function renderStubs(array $pairs): string // ShadowedByTurboExtension attribute (plus the hardcoded vendored // PhpParser\NodeTraverser) with the phpstan_turbo extension's native // implementation. Required by PHPStan\Turbo\TurboExtensionEnabler before -// the Composer autoloader registers. +// the Composer autoloader registers — hence the interface sources required +// below, which the autoloader cannot resolve yet. %s diff --git a/build/generate-turbo-stubs.php b/build/generate-turbo-stubs.php index ecd98134bc9..7fa288f4277 100644 --- a/build/generate-turbo-stubs.php +++ b/build/generate-turbo-stubs.php @@ -7,7 +7,8 @@ * * - vendor/turbo-stubs.php: one empty stub shell per shadowed class, * extending the phpstan_turbo extension's native counterpart the attribute - * names. TurboExtensionEnabler requires the file before the Composer + * names and repeating the class's own implements clause. + * TurboExtensionEnabler requires the file before the Composer * autoloader registers, so with the extension active every reference to * the original class name transparently resolves to the native * implementation. diff --git a/src/Turbo/ShadowedByTurboExtension.php b/src/Turbo/ShadowedByTurboExtension.php index 45b1ed6e2c7..adaedecc627 100644 --- a/src/Turbo/ShadowedByTurboExtension.php +++ b/src/Turbo/ShadowedByTurboExtension.php @@ -12,7 +12,9 @@ * * On composer dump-autoload, build/generate-turbo-stubs.php collects these * attributes and generates vendor/turbo-stubs.php — an empty stub shell per - * class extending the native one — which TurboExtensionEnabler declares + * class extending the native one and repeating its implements clause (a + * parent class is rejected: the shell has no inheritance slot left for it) + * — which TurboExtensionEnabler declares * before the Composer autoloader registers when the extension is active, * plus vendor/turbo-shadowed-classes.json — the manifest of shadowed pairs * read by the enabler, the compiler's preload builder, and the parity diff --git a/turbo-ext/CLAUDE.md b/turbo-ext/CLAUDE.md index 35bd68784c9..5ac6b02b315 100644 --- a/turbo-ext/CLAUDE.md +++ b/turbo-ext/CLAUDE.md @@ -21,9 +21,12 @@ being ≥0.5% faster is. When the estimate is marginal, don't port. `grep "\$this->foo"` in double quotes sends `\$` to grep and silently matches nothing — use single quotes). Run the full test suite now, before any native work. -2. **Check it is not a DI service** (rule 6 in README — this is fatal and - was measured at 617 broken tests). Value classes and manually-instantiated - classes are safe. +2. **Check it has no parent class** — the stub shell extends the native + class and PHP is single-inheritance, so the collector rejects it. If it is + a DI service, its native `__construct` arginfo must declare the real + parameter class names (rule 6 in README): Nette autowires by reflecting + the constructor, and erased types fail container compilation for every + shadowed service at once. 3. **Implement natively**: one class per `.cpp` in `src/`, namespace `PHPStanTurbo`, class **non-final**, `instanceof`-style checks instead of exact class-entry comparisons. Hot classes are registered with the raw diff --git a/turbo-ext/README.md b/turbo-ext/README.md index a62acb3b47c..630b721846f 100644 --- a/turbo-ext/README.md +++ b/turbo-ext/README.md @@ -56,6 +56,12 @@ Every shadowed piece of PHP code follows the same three steps: per class, `final class Foo extends \PHPStanTurbo\Foo {}` (shadowed classes living in vendor/ cannot carry the attribute and are hardcoded in `build/TurboAttributeCollector.php`; currently `PhpParser\NodeTraverser`). + The shell repeats the class's own `implements` clause, because it inherits + from the native class alone — an interface left off it silently stops + matching `instanceof` and DI type lookups — and `require`s the interface + sources, which the autoloader cannot yet provide at that point. A parent + class cannot survive the same way (PHP is single-inheritance), so the + collector rejects a shadowed class that has one. When the extension is enabled, `PHPStan\Turbo\TurboExtensionEnabler` `require`s that file *before* the Composer autoloader registers. All PHP code keeps calling @@ -336,9 +342,13 @@ Measured in the July 2026 benchmarks (callback-free absorptions gained call is exactly the per-element boundary cost these rules forbid. (The extension originally hosted its lifecycle in PHP-CPP; it is a plain Zend module since the Windows port.) -6. **Never shadow a DI-service class.** Nette's `getByType()` normalizes - requested types through reflection to the real class name and breaks - containers cached in the other mode. +6. **A shadowed DI-service class has to stay autowirable.** Nette reflects + `__construct` while it compiles the container, so the native arginfo must + declare the real parameter class names — erasing them fails container + compilation for every shadowed service at once. (Aliasing a service class + is what genuinely cannot work: `getByType()` normalizes the requested type + through reflection to the real class name. The stub shells are subclasses + carrying the original name, so that does not apply to them.) 7. **Every port must prove itself**: interleaved A/B benchmark on a long run (user CPU, result cache cleared) plus a byte-identical output diff. Ports measuring ≤0.5% get reverted — the failure mode is silent no-gain, and diff --git a/turbo-ext/src/NodeTraverser.cpp b/turbo-ext/src/NodeTraverser.cpp index 22b7cb0aa20..4c6a991646e 100644 --- a/turbo-ext/src/NodeTraverser.cpp +++ b/turbo-ext/src/NodeTraverser.cpp @@ -877,13 +877,16 @@ void pt_register_node_traverser() } }); + static const reg::Arg voidReturn = { "", MAY_BE_VOID, nullptr }; + static const reg::Arg arrayReturn = reg::arrayArg(""); + cls.method("addVisitor", reg::Public, 1, { reg::obj("visitor", NODE_VISITOR_CLASS) }, [](INTERNAL_FUNCTION_PARAMETERS) { zval *visitor; ZEND_PARSE_PARAMETERS_START(1, 1) Z_PARAM_OBJECT(visitor) ZEND_PARSE_PARAMETERS_END(); NodeTraverser(Z_OBJ_P(ZEND_THIS)).addVisitor(zv::Ref(visitor)); - }); + }, &voidReturn); cls.method("removeVisitor", reg::Public, 1, { reg::obj("visitor", NODE_VISITOR_CLASS) }, [](INTERNAL_FUNCTION_PARAMETERS) { zval *visitor; @@ -891,7 +894,7 @@ void pt_register_node_traverser() Z_PARAM_OBJECT(visitor) ZEND_PARSE_PARAMETERS_END(); NodeTraverser(Z_OBJ_P(ZEND_THIS)).removeVisitor(zv::Ref(visitor)); - }); + }, &voidReturn); cls.method("traverse", reg::Public, 1, { reg::arrayArg("nodes") }, [](INTERNAL_FUNCTION_PARAMETERS) { HashTable *nodes; @@ -904,7 +907,7 @@ void pt_register_node_traverser() RETURN_THROWS(); } result.intoReturnValue(return_value); - }); + }, &arrayReturn); pt_ce_node_traverser = cls.register_(); } diff --git a/turbo-ext/src/reg.h b/turbo-ext/src/reg.h index dc103002dba..2e0c2f561f1 100644 --- a/turbo-ext/src/reg.h +++ b/turbo-ext/src/reg.h @@ -116,14 +116,16 @@ class Class * requiredArgs is ZEND_BEGIN_ARG_INFO_EX's required_num_args; args carry * name/type/by-ref/variadic exactly as the ZEND_ARG_* macros would. */ - Class &method(const char *methodName, uint32_t flags, uint32_t requiredArgs, std::initializer_list args, zif_handler handler) + Class &method(const char *methodName, uint32_t flags, uint32_t requiredArgs, std::initializer_list args, zif_handler handler, const Arg *returns = NULL) { /* arginfo array: slot 0 is the return-info slot carrying the - * required-args count, exactly as ZEND_BEGIN_ARG_INFO_EX emits */ + * required-args count, exactly as ZEND_BEGIN_ARG_INFO_EX emits. + * A declared return type goes in the same slot's type — needed only + * where the engine enforces it (implementing a userland interface). */ auto *argInfo = (zend_internal_arg_info *) pemalloc(sizeof(zend_internal_arg_info) * (args.size() + 1), 1); argInfo[0].name = (const char *) (uintptr_t) requiredArgs; - argInfo[0].type.ptr = NULL; - argInfo[0].type.type_mask = 0; + argInfo[0].type.ptr = returns != NULL ? (void *) returns->className : NULL; + argInfo[0].type.type_mask = returns != NULL ? returns->typeMask : 0; argInfo[0].default_value = NULL; size_t i = 1; for (const Arg &arg : args) { From 624a9c408496e44f7ae634bbd409b2b80bfd7130 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 27 Jul 2026 18:00:40 +0200 Subject: [PATCH 2/3] Bump expected turbo version --- src/Turbo/TurboExtensionEnabler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index c41df8b63c8..691c31e90ce 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -20,7 +20,7 @@ final class TurboExtensionEnabler * version is the short SHA of the last commit touching turbo-ext/src/, * enforced by the phar.yml turbo-version job. */ - public const EXPECTED_EXTENSION_VERSION = '2d4b432'; + public const EXPECTED_EXTENSION_VERSION = 'e9ce4dc'; private static bool $typeCombinatorCacheEnabled = false; From 030b59fee31e43f786f0ddf05a1c3c75feac9a96 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 27 Jul 2026 17:57:22 +0200 Subject: [PATCH 3/3] Key the container cache on the turbo stubs being active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The key used TurboExtensionEnabler::isLoaded(), which is true whenever the extension is present — including when enableIfLoaded() declined to declare the stubs because the version did not match. A container compiled while the stubs shadowed a service class would then be reused by a run reflecting the PHP implementations instead. isActive() is the state the compiled container actually depends on. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016XhKccs7xeB1wRTAQEemc2 --- src/DependencyInjection/Configurator.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/DependencyInjection/Configurator.php b/src/DependencyInjection/Configurator.php index 7865956266b..d31fa7e97af 100644 --- a/src/DependencyInjection/Configurator.php +++ b/src/DependencyInjection/Configurator.php @@ -105,7 +105,12 @@ public function loadContainer(): string is_file($attributesPhp) ? hash_file('sha256', $attributesPhp) : 'attributes-missing', NeonAdapter::CACHE_KEY, $this->getAllConfigFilesHashes(), - var_export(TurboExtensionEnabler::isLoaded(), true), + // the stubs, not the extension: a shadowed service class is + // reflected while the container compiles, so a container built + // against the stub shells must not be reused by a run that loaded + // the extension but left it inactive (version mismatch, + // PHPSTAN_TURBO=0) and reflects the PHP implementations instead + var_export(TurboExtensionEnabler::isActive(), true), ]; $className = $loader->load(