fix(translator): correct trait composition precedence and alias modifiers - #19
Open
AlessioGiacobbe wants to merge 1 commit into
Open
fix(translator): correct trait composition precedence and alias modifiers#19AlessioGiacobbe wants to merge 1 commit into
AlessioGiacobbe wants to merge 1 commit into
Conversation
…iers
Three defects in composeTraitAst diverged from PHP trait semantics:
1. When a later trait supplied a concrete method for a name an earlier
trait declared abstract, the branch unset the new concrete statement
while the already-merged abstract one stayed in the class AST. The
composed class kept only the abstract declaration, so instantiating
it failed with "Cannot instantiate abstract class" even though the
method body existed. The reverse trait order worked. Now the merged
abstract declaration is dropped and the concrete method is kept.
2. Alias adaptations assigned the new modifier over the whole flag set,
wiping static/final/abstract: `use Maker { make as protected; }`
turned a static method into an instance method, and static:: calls
on it miscompiled. PHP replaces only the visibility bits (and keeps
the original visibility when the modifier carries none, e.g.
`as final`).
3. A method defined by the class itself did not suppress the
trait-vs-trait conflict check, so `class C { use A, B; function f(){} }`
with f() in both traits died with a spurious "method already exists"
fatal. The class-method check now runs before conflict resolution and
suppressed trait copies are no longer registered as trait methods.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Three defects in
composeTraitAstdiverge from PHP trait semantics:1. A concrete method fulfilling another trait's abstract requirement is dropped (order-dependent):
The branch unsets the new concrete statement while the already-merged abstract one stays in the class AST, so the compiled class keeps only the abstract declaration →
Cannot instantiate abstract class Cat runtime.use HasName, NeedsName;(reverse order) works.2. Alias adaptations wipe
static/final/abstract:$traitStmt->flags = $alias['newModifier']replaces the whole flag set with just the visibility bits, somakelosesstaticand compiles as an instance method. PHP replaces only the visibility bits (verified via Reflection:make as protectedkeepsisStatic(), andm as finalkeeps the original visibility).3. A class-defined method doesn't suppress trait-vs-trait conflicts:
dies with the spurious fatal
Trait `B` method `who` already existsbecause the class-method check ran after conflict resolution, and suppressed trait copies were still registered as trait methods.Fix
applyTraitAliasModifier(): a new visibility replaces onlyModifiers::VISIBILITY_MASKbits; a modifier without visibility keeps the original one.$traitMethods.Tests
tests/compiler/trait/trait-composition-precedence.phptcovers all three (both trait orders for the abstract case, alias visibility change with same and new name calling throughstatic::, class method winning over two conflicting traits). Expected output verified against PHP 8.4.tests/compiler/trait/suite: 41/41 pass. No new PHPStan errors onTranslator.php.Trait `WhoB` method `who` already existsat compile time and, with that scenario removed,Cannot instantiate abstract class AbstractFirstat runtime.