fix(optimizer): preserve typed builtin argument validation - #18
Conversation
matyhtf
left a comment
There was a problem hiding this comment.
Thank you for the thorough investigation and for preserving the whole-call fallback path. The original mixed argument issue is real, and this PR fixes that case in the right direction.
I found two remaining correctness issues in hasOptimizerSafeTypedArguments() that need to be addressed before merging:
- Treating every Native scalar-to-scalar conversion as safe is too broad.
declare(strict_types=1);
function typedInt(): int
{
return 1;
}
in_array('1', [1], typedInt());Zend throws a TypeError, but this PR emits php::toBool(...) and returns false. A declared Native return type does not make a cross-type conversion valid for an internal function. The fast path should normally require an exact type, with only explicitly proven Zend-compatible widening such as int -> float; otherwise the whole call should fall back to Zend dispatch.
- Literal
nullcannot be accepted unconditionally.
declare(strict_types=1);
strlen(null);Zend throws a TypeError, while this PR emits php::toString(php::null) and returns 0. Please use the reflected nullable metadata when deciding whether null is safe. If a particular wrapper intentionally accepts null as a legacy/default policy, that exception should be explicit per function/parameter rather than globally applied.
I reproduced both differences by compiling the PR head and comparing the resulting executable with Zend PHP. These are implementation issues rather than requests for additional test coverage.
The macOS PHP 8.5 CI failure appears unrelated: Composer failed because Packagist DNS resolution timed out. The remaining CI jobs passed.
Summary
string,int,float,bool, orarrayvalue still needs PHP's parameter validationInvariant
An optimizer must not erase the runtime zval type before an internal function has applied its parameter contract. A direct ABI call is valid only when the compiler proves that its conversion has the same accepted/rejected behavior as Zend; otherwise all arguments remain on the ordered
php::call()path.This fixes strict calls such as a
mixedvalue passed toin_array()'s boolean parameter being coerced by generated C++ instead of raising the sameTypeErroras Zend.Coverage
string,int,float,bool, andarrayABI parametersarray_keys,array_key_exists,round,count,define, andfunction_exists)round(), and nativeCountablepaths remain optimizedVerification
array_keys, lookup calls,func_get_arg, Decimalround,count(Countable), and internal-interface coverage)bindec()deprecations and Windows path separators)The full PHPUnit suite was also attempted, but the current Windows run has unrelated existing failures and terminates on a Linux-only
profilefixture, so it is not claimed as passing. Remote CI and non-MSVC native artifacts were not run locally.