fix(optimizer): class_exists() must not fold a trait name to true - #26
Merged
Merged
Conversation
doFoldKnownClass folds class_exists() whenever the name is a literal the
symbol table knows. That table also holds traits, so a trait name folded
to true while PHP answers false:
trait Helper {}
class_exists('Helper'); // folded to true
$name = 'Helper';
class_exists($name); // reaches php::fn::class_exists, answers false
The same program therefore gives two different answers for the same
trait, decided only by whether the argument is a literal.
The runtime side is already right, and deliberately so: traits are
compile-time AST templates in TypePHP, which is why
tests/compiler/stdlib/class_exists.phpt expects trait_exists() to be
false. Only the constant fold disagreed - with PHP and with the
compiler's own runtime.
A trait name now folds to false. Classes and enums keep folding to true,
which matches PHP: an enum is a class, a trait is not.
class_exists.phpt gains the literal and non-literal trait cases, and
ClassExistsTraitFoldTest pins the fold decision in the generated C++.
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
2 participants
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.
Fixes #25
doFoldKnownClassfoldsclass_exists()whenever the symbol table knows theliteral name. Traits live in that table, so a trait folded to
true:PHP answers
falsefor both. So does this compiler's runtime - traits arecompile-time AST templates here, which is why
tests/compiler/stdlib/class_exists.phptalready expectstrait_exists()to befalse. Only the fold disagreed, and only for a literal argument.The change
A trait name folds to
false. Classes and enums are untouched and keep foldingto
true, matching PHP, where an enum is a class and a trait is not. Interfacesalready fell through to the runtime call, so they are unaffected.
Tests
tests/compiler/stdlib/class_exists.phptgains the two missing cases: thetrait name as a literal and through a variable. The file already covered
class_existsfor a class andtrait_existsfor a trait, but neverclass_existsfor a trait.phpunit/src/ClassExistsTraitFoldTest.phppins the fold decision in thegenerated C++ and runs without a native toolchain.
The PHPUnit test fails on master and passes with the change. The full suite
reports the same results as master, and PHPStan reports no new findings for the
touched file.