Skip to content

fix(compiler): support the single-argument implode() form - #158

Merged
matyhtf merged 2 commits into
swoole:masterfrom
alwaysLinger:fix/implode-single-arg
Oct 13, 2026
Merged

matyhtf merged 2 commits into
swoole:masterfrom
alwaysLinger:fix/implode-single-arg

Conversation

@alwaysLinger

Copy link
Copy Markdown
Contributor

Observed failure

PHP's documented single-argument form implode(array $array) (php.net lists it as an official alternative signature; the separator defaults to "") fails to compile:

<?php
function main(): void
{
    var_dump(implode(['a', 'b']));
}

ZendPHP 8.4:

string(2) "ab"

TypePHP master (c481e06):

/usr/local/bin/ld: .../n3.cc:26:33: error: too few arguments to function
  'php::String php::fn::implode(const php::String&, const php::Array&)'
Fatal error: Compilation failed

The two-argument form implode('-', $arr) works, so the failure is specific to the arity-1 call. The join() alias shares the same code path and is equally affected.

Root cause

phpx declares only the two-argument overload (vendor/swoole/phpx/include/std/string.h):

String implode(const String &glue, const Array &pieces);

PHP's reflection reports implode() as requiring only 1 parameter (getNumberOfRequiredParameters() === 1, verified), so TypePHP's dispatchFuncCall() arity gate passes a single-argument call. buildArgList() then walks the two-entry reflection type list against one provided argument and emits php::fn::implode(<array>) — a call the C++ side cannot resolve.

Fix

src/Optimizer/FuncCallOptimizer.php, in dispatchFuncCall(): when implode is called with exactly one argument, dispatch through the existing two-argument overload with an empty glue string:

if ($name === 'implode' && count($expr->args) === 1) {
    return $target . '("", ' . $this->resolveArg($expr, 0, self::ARG_TYPE_ARRAY) . ')';
}

The array argument is converted by the same resolveArg() machinery a normal two-argument call uses (ARG_TYPE_ARRAY), and the empty glue is emitted as a raw C++ literal — the same shape a literal two-argument call produces. The join() alias resolves to implode before this point, so both names are fixed. The two-argument path is untouched.

@matyhtf matyhtf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single-argument array cases are fixed, but the new branch bypasses PHP's parameter validation by converting arbitrary values to arrays. Please address this before merging.

Reproducer tested against PR HEAD e746946fd66548e7184412e25e58e8ebc9c44703:

function test(mixed $value): string {
    return implode($value);
}

function main(): void {
    var_dump(test(42));
}

PHP throws TypeError; the PR build prints string(2) "42". Strings, booleans, null, and objects are also incorrectly accepted. The join() alias has the same issue. Even a direct implode('ab') returns "ab" instead of throwing.

The new resolveArg(..., ARG_TYPE_ARRAY) call generates:

php::fn::implode("", php::toArray(value))

Reflection describes the first parameter as array|string, so the existing argument safety check does not enforce the array-only contract of the single-argument form. Calling toArray() then erases the original type before Zend can reject it.

Please:

  1. Validate optimizer safety against the single-argument form's actual array contract. Only optimize arguments that are statically proven to be arrays; otherwise fall back to Zend's normal parameter validation.
  2. Add PHPT coverage for invalid direct arguments and invalid values passed through mixed/var, for both implode() and join(). Include strings, integers, booleans, null, and objects, and retain coverage for valid arrays passed through a dynamic value.

Validation: all 74 stdlib PHPTs and 3 related PHPUnit tests pass on this PR. The added PHPT fails to compile on the base and passes on the PR. Additional valid named-argument, unpacking, dynamic-value, and single-evaluation cases match PHP. The existing tests do not cover the invalid-input cases above.

Thanks for the contribution and the regression test.

@alwaysLinger

Copy link
Copy Markdown
Contributor Author

Both points addressed:

  1. The optimizer rewrite is now gated on the argument being statically
    proven array (detectTypeOfExpr() === Type::ARRAY). Everything else
    — including values passed through mixed/var — falls back to
    php::call(), so Zend performs its normal parameter validation and
    throws the expected TypeError (your reproducer now reports
    TypeError on the PR head).

  2. Added PHPT coverage for invalid arguments: integers, strings,
    booleans, null, and objects passed both directly and through
    mixed/var, for both implode() and join(), each expecting a
    TypeError. Also retained and extended the valid-array coverage:
    arrays passed through dynamic values (std::any(), mixed
    parameters) still join correctly, and named-argument and unpacked
    single-argument calls behave identically to master.

Full tests/compiler suite: 1291 passed, no new failures.

@matyhtf
matyhtf merged commit 8e432a8 into swoole:master Oct 13, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants