Skip to content

Commit a869b9c

Browse files
alganetalganet
committed
Align array_diff* functions to official PHP behavior
1 parent 47dc4cc commit a869b9c

32 files changed

+696
-69
lines changed

‎src/ph7/hashmap.c‎

Lines changed: 216 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -3714,10 +3714,32 @@ static int ph7_hashmap_diff(ph7_context *pCtx,int nArg,ph7_value **apArg)
37143714
sxi32 rc;
37153715
sxu32 n;
37163716
int i;
3717-
if( nArg < 1 || !ph7_value_is_array(apArg[0]) ){
3718-
/* Missing arguments,return NULL */
3719-
ph7_result_null(pCtx);
3720-
return PH7_OK;
3717+
/* Validate arguments to mimic PHP behaviour. Earlier versions simply
3718+
* returned NULL when the caller passed invalid parameters which made
3719+
* debugging difficult. */
3720+
if( nArg < 1 ){
3721+
return PH7_VmThrowException(pCtx,
3722+
"ArgumentCountError",
3723+
"array_diff() expects at least 1 argument, %d given",
3724+
nArg
3725+
);
3726+
}
3727+
if( !ph7_value_is_array(apArg[0]) ){
3728+
return PH7_VmThrowException(pCtx,
3729+
"TypeError",
3730+
"array_diff(): Argument #1 ($array) must be of type array, %s given",
3731+
ph7_type_name(apArg[0])
3732+
);
3733+
}
3734+
for(i = 1 ; i < nArg ; i++){
3735+
if( !ph7_value_is_array(apArg[i]) ){
3736+
return PH7_VmThrowException(pCtx,
3737+
"TypeError",
3738+
"array_diff(): Argument #%d must be of type array, %s given",
3739+
i + 1,
3740+
ph7_type_name(apArg[i])
3741+
);
3742+
}
37213743
}
37223744
if( nArg == 1 ){
37233745
/* Return the first array since we cannot perform a diff */
@@ -3879,10 +3901,32 @@ static int ph7_hashmap_diff_assoc(ph7_context *pCtx,int nArg,ph7_value **apArg)
38793901
sxi32 rc;
38803902
sxu32 n;
38813903
int i;
3882-
if( nArg < 1 || !ph7_value_is_array(apArg[0]) ){
3883-
/* Missing arguments,return NULL */
3884-
ph7_result_null(pCtx);
3885-
return PH7_OK;
3904+
/* Ensure the argument list is valid, emitting the same errors PHP
3905+
* would produce. This makes behaviour predictable and allows the
3906+
* accompanying integration tests to pass. */
3907+
if( nArg < 1 ){
3908+
return PH7_VmThrowException(pCtx,
3909+
"ArgumentCountError",
3910+
"array_diff_assoc() expects at least 1 argument, %d given",
3911+
nArg
3912+
);
3913+
}
3914+
if( !ph7_value_is_array(apArg[0]) ){
3915+
return PH7_VmThrowException(pCtx,
3916+
"TypeError",
3917+
"array_diff_assoc(): Argument #1 ($array) must be of type array, %s given",
3918+
ph7_type_name(apArg[0])
3919+
);
3920+
}
3921+
for(i = 1 ; i < nArg ; i++){
3922+
if( !ph7_value_is_array(apArg[i]) ){
3923+
return PH7_VmThrowException(pCtx,
3924+
"TypeError",
3925+
"array_diff_assoc(): Argument #%d must be of type array, %s given",
3926+
i + 1,
3927+
ph7_type_name(apArg[i])
3928+
);
3929+
}
38863930
}
38873931
if( nArg == 1 ){
38883932
/* Return the first array since we cannot perform a diff */
@@ -3902,15 +3946,14 @@ static int ph7_hashmap_diff_assoc(ph7_context *pCtx,int nArg,ph7_value **apArg)
39023946
n = pSrc->nEntry;
39033947
pN1 = pN2 = 0;
39043948
for(;;){
3949+
int keep;
39053950
if( n < 1 ){
39063951
break;
39073952
}
3953+
/* assume the element should be kept until we find a match */
3954+
keep = 1;
39083955
for( i = 1 ; i < nArg ; i++ ){
3909-
if( !ph7_value_is_array(apArg[i])) {
3910-
/* ignore */
3911-
continue;
3912-
}
3913-
/* Point to the internal representation of the hashmap */
3956+
/* all arguments have been validated already, so cast directly */
39143957
pMap = (ph7_hashmap *)apArg[i]->x.pOther;
39153958
/* Perform a key lookup first */
39163959
if( pEntry->iType == HASHMAP_INT_NODE ){
@@ -3919,21 +3962,25 @@ static int ph7_hashmap_diff_assoc(ph7_context *pCtx,int nArg,ph7_value **apArg)
39193962
rc = HashmapLookupBlobKey(pMap,SyBlobData(&pEntry->xKey.sKey),SyBlobLength(&pEntry->xKey.sKey),&pN1);
39203963
}
39213964
if( rc != SXRET_OK ){
3922-
/* No such key,break immediately */
3923-
break;
3965+
/* this array does not contain the key, continue checking others */
3966+
continue;
39243967
}
3925-
/* Extract node value */
3968+
/* key exists; check that value stored in the matching node is equal */
39263969
pVal = HashmapExtractNodeValue(pEntry);
39273970
if( pVal ){
3928-
/* Perform the lookup */
3929-
rc = HashmapFindValue(pMap,pVal,&pN2,TRUE);
3930-
if( rc != SXRET_OK || pN1 != pN2 ){
3931-
/* Value does not exist */
3932-
break;
3971+
/* directly compare with value at pN1 rather than searching again */
3972+
ph7_value *pVal2 = HashmapExtractNodeValue(pN1);
3973+
if( pVal2 ){
3974+
sxi32 cmp = PH7_MemObjCmp(pVal, pVal2, TRUE, 0);
3975+
if( cmp == 0 ){
3976+
/* identical key+value found in one of the arrays => drop it */
3977+
keep = 0;
3978+
break;
3979+
}
39333980
}
39343981
}
39353982
}
3936-
if( i < nArg ){
3983+
if( keep ){
39373984
/* Perform the insertion */
39383985
HashmapInsertNode((ph7_hashmap *)pArray->x.pOther,pEntry,TRUE);
39393986
}
@@ -3966,24 +4013,75 @@ static int ph7_hashmap_diff_assoc(ph7_context *pCtx,int nArg,ph7_value **apArg)
39664013
*/
39674014
static int ph7_hashmap_diff_uassoc(ph7_context *pCtx,int nArg,ph7_value **apArg)
39684015
{
3969-
ph7_hashmap_node *pN1,*pN2,*pEntry;
4016+
ph7_hashmap_node *pEntry;
39704017
ph7_hashmap *pSrc,*pMap;
39714018
ph7_value *pCallback;
39724019
ph7_value *pArray;
3973-
ph7_value *pVal;
39744020
sxi32 rc;
39754021
sxu32 n;
39764022
int i;
39774023

3978-
if( nArg < 2 || !ph7_value_is_array(apArg[0]) ){
3979-
/* Missing/Invalid arguments,return NULL */
3980-
ph7_result_null(pCtx);
3981-
return PH7_OK;
4024+
/* Argument validation mimicking PHP errors. */
4025+
if( nArg < 2 ){
4026+
return PH7_VmThrowException(pCtx,
4027+
"ArgumentCountError",
4028+
"array_diff_uassoc() expects at least 2 arguments, %d given",
4029+
nArg
4030+
);
39824031
}
3983-
/* Point to the callback */
4032+
if( !ph7_value_is_array(apArg[0]) ){
4033+
return PH7_VmThrowException(pCtx,
4034+
"TypeError",
4035+
"array_diff_uassoc(): Argument #1 ($array) must be of type array, %s given",
4036+
ph7_type_name(apArg[0])
4037+
);
4038+
}
4039+
/* Intermediate arguments (except last) must be arrays. Last argument is
4040+
* expected to be a callback. */
4041+
for(i = 1 ; i < nArg - 1; i++){
4042+
if( !ph7_value_is_array(apArg[i]) ){
4043+
return PH7_VmThrowException(pCtx,
4044+
"TypeError",
4045+
"array_diff_uassoc(): Argument #%d must be of type array, %s given",
4046+
i + 1,
4047+
ph7_type_name(apArg[i])
4048+
);
4049+
}
4050+
}
4051+
/* Point to the callback value */
39844052
pCallback = apArg[nArg - 1];
4053+
if( !ph7_value_is_callable(pCallback) ){
4054+
/* Compose an error message that closely matches PHP output. When the
4055+
* argument is an array of the wrong shape we include an extra clause.
4056+
* If the value is neither array nor string, PHP says "no array or
4057+
* string given" which we also reproduce. */
4058+
if( ph7_value_is_array(pCallback) ){
4059+
/* ARRAY CALLBACK must have exactly two members */
4060+
return PH7_VmThrowException(pCtx,
4061+
"TypeError",
4062+
"array_diff_uassoc(): Argument #%d must be a valid callback, array callback must have exactly two members",
4063+
nArg
4064+
);
4065+
}
4066+
if( !ph7_value_is_string(pCallback) ){
4067+
/* neither array nor string */
4068+
return PH7_VmThrowException(pCtx,
4069+
"TypeError",
4070+
"array_diff_uassoc(): Argument #%d must be a valid callback, no array or string given",
4071+
nArg
4072+
);
4073+
}
4074+
/* Fallback for string (non-callable) or other leftover cases */
4075+
return PH7_VmThrowException(pCtx,
4076+
"TypeError",
4077+
"array_diff_uassoc(): Argument #%d must be a valid callback, %s given",
4078+
nArg,
4079+
ph7_type_name(pCallback)
4080+
);
4081+
}
39854082
if( nArg == 2 ){
3986-
/* Return the first array since we cannot perform a diff */
4083+
/* If we only have the first array and the callback, just return the
4084+
* input array. */
39874085
ph7_result_value(pCtx,apArg[0]);
39884086
return PH7_OK;
39894087
}
@@ -3998,40 +4096,77 @@ static int ph7_hashmap_diff_uassoc(ph7_context *pCtx,int nArg,ph7_value **apArg)
39984096
/* Perform the diff */
39994097
pEntry = pSrc->pFirst;
40004098
n = pSrc->nEntry;
4001-
pN1 = pN2 = 0; /* cc warning */
40024099
for(;;){
4100+
int keep;
40034101
if( n < 1 ){
40044102
break;
40054103
}
4104+
keep = 1;
40064105
for( i = 1 ; i < nArg - 1; i++ ){
4007-
if( !ph7_value_is_array(apArg[i])) {
4008-
/* ignore */
4009-
continue;
4010-
}
4011-
/* Point to the internal representation of the hashmap */
4106+
/* each of these must already be arrays thanks to earlier validation */
40124107
pMap = (ph7_hashmap *)apArg[i]->x.pOther;
4013-
/* Perform a key lookup first */
4014-
if( pEntry->iType == HASHMAP_INT_NODE ){
4015-
rc = HashmapLookupIntKey(pMap,pEntry->xKey.iKey,&pN1);
4016-
}else{
4017-
rc = HashmapLookupBlobKey(pMap,SyBlobData(&pEntry->xKey.sKey),SyBlobLength(&pEntry->xKey.sKey),&pN1);
4018-
}
4019-
if( rc != SXRET_OK ){
4020-
/* No such key,break immediately */
4021-
break;
4022-
}
4023-
/* Extract node value */
4024-
pVal = HashmapExtractNodeValue(pEntry);
4025-
if( pVal ){
4026-
/* Invoke the user callback */
4027-
rc = HashmapFindValueByCallback(pMap,pVal,pCallback,&pN2);
4028-
if( rc != SXRET_OK || pN1 != pN2 ){
4029-
/* Value does not exist */
4030-
break;
4108+
/* we must compare keys via callback, not by direct lookup */
4109+
ph7_hashmap_node *pIt = pMap->pFirst;
4110+
while( pIt ){
4111+
/* build temporary key values for callback */
4112+
ph7_value key1, key2, result;
4113+
/* initialise only once using the appropriate helper */
4114+
if( pEntry->iType == HASHMAP_INT_NODE ){
4115+
PH7_MemObjInitFromInt(pMap->pVm,&key1,pEntry->xKey.iKey);
4116+
}else{
4117+
SyString sStr;
4118+
SyStringInitFromBuf(&sStr,
4119+
SyBlobData(&pEntry->xKey.sKey),
4120+
SyBlobLength(&pEntry->xKey.sKey));
4121+
PH7_MemObjInitFromString(pMap->pVm,&key1,&sStr);
4122+
}
4123+
if( pIt->iType == HASHMAP_INT_NODE ){
4124+
PH7_MemObjInitFromInt(pMap->pVm,&key2,pIt->xKey.iKey);
4125+
}else{
4126+
SyString sStr;
4127+
SyStringInitFromBuf(&sStr,
4128+
SyBlobData(&pIt->xKey.sKey),
4129+
SyBlobLength(&pIt->xKey.sKey));
4130+
PH7_MemObjInitFromString(pMap->pVm,&key2,&sStr);
4131+
}
4132+
PH7_MemObjInit(pMap->pVm,&result);
4133+
/* call user callback with (key1, key2) */
4134+
{
4135+
ph7_value *apK[2];
4136+
apK[0] = &key1;
4137+
apK[1] = &key2;
4138+
rc = PH7_VmCallUserFunction(pMap->pVm,pCallback,2,apK,&result);
40314139
}
4140+
if( rc == SXRET_OK ){
4141+
if( (result.iFlags & MEMOBJ_INT) == 0 ){
4142+
PH7_MemObjToInteger(&result);
4143+
}
4144+
if( result.x.iVal == 0 ){
4145+
/* keys considered equal by callback; now compare values */
4146+
ph7_value *pVal1 = HashmapExtractNodeValue(pEntry);
4147+
ph7_value *pVal2 = HashmapExtractNodeValue(pIt);
4148+
if( pVal1 && pVal2 ){
4149+
if( PH7_MemObjCmp(pVal1,pVal2,TRUE,0) == 0 ){
4150+
keep = 0;
4151+
PH7_MemObjRelease(&result);
4152+
/* release keys too before breaking */
4153+
PH7_MemObjRelease(&key1);
4154+
PH7_MemObjRelease(&key2);
4155+
break;
4156+
}
4157+
}
4158+
}
4159+
}
4160+
PH7_MemObjRelease(&result);
4161+
PH7_MemObjRelease(&key1);
4162+
PH7_MemObjRelease(&key2);
4163+
/* move to next node */
4164+
pIt = pIt->pPrev;
4165+
if( keep == 0 ) break;
40324166
}
4167+
if( keep == 0 ) break;
40334168
}
4034-
if( i < (nArg-1) ){
4169+
if( keep ){
40354170
/* Perform the insertion */
40364171
HashmapInsertNode((ph7_hashmap *)pArray->x.pOther,pEntry,TRUE);
40374172
}
@@ -4066,10 +4201,32 @@ static int ph7_hashmap_diff_key(ph7_context *pCtx,int nArg,ph7_value **apArg)
40664201
sxi32 rc;
40674202
sxu32 n;
40684203
int i;
4069-
if( nArg < 1 || !ph7_value_is_array(apArg[0]) ){
4070-
/* Missing arguments,return NULL */
4071-
ph7_result_null(pCtx);
4072-
return PH7_OK;
4204+
/* Validate arguments to mirror PHP behaviour. Previously invalid inputs
4205+
* would quietly return NULL which is inconsistent with other hashmap
4206+
* helpers. */
4207+
if( nArg < 1 ){
4208+
return PH7_VmThrowException(pCtx,
4209+
"ArgumentCountError",
4210+
"array_diff_key() expects at least 1 argument, %d given",
4211+
nArg
4212+
);
4213+
}
4214+
if( !ph7_value_is_array(apArg[0]) ){
4215+
return PH7_VmThrowException(pCtx,
4216+
"TypeError",
4217+
"array_diff_key(): Argument #1 ($array) must be of type array, %s given",
4218+
ph7_type_name(apArg[0])
4219+
);
4220+
}
4221+
for(i = 1 ; i < nArg ; i++){
4222+
if( !ph7_value_is_array(apArg[i]) ){
4223+
return PH7_VmThrowException(pCtx,
4224+
"TypeError",
4225+
"array_diff_key(): Argument #%d must be of type array, %s given",
4226+
i + 1,
4227+
ph7_type_name(apArg[i])
4228+
);
4229+
}
40734230
}
40744231
if( nArg == 1 ){
40754232
/* Return the first array since we cannot perform a diff */

‎tests/ph7/001-smoke/function/array_diff/array_diff_basic.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2025 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_diff should return expected keys and preserve keys across associative arrays
6+
--FILE--
7+
<?php
8+
$a = array('a' => 1, 'b' => 2, 'c' => 3);
9+
$b = array('b' => 2);
10+
// array_diff only compares values
11+
$c = array_diff($a, $b);
12+
echo implode(',', array_keys($c)) . PHP_EOL; // expecting 'a,c'
13+
?>
14+
--EXPECT--
15+
a,c
16+
--CLEAN--
17+
<?php
18+
unset($a, $b, $c);

‎tests/ph7/001-smoke/function/array_diff/array_diff_multiple.phpt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--CREDITS--
2+
SPDX-FileCopyrightText: 2026 Alexandre Gomes Gaigalas <alganet@gmail.com>
3+
SPDX-License-Identifier: BSD-3-Clause
4+
--TEST--
5+
array_diff with multiple arrays should only keep values absent from all others
6+
--FILE--
7+
<?php
8+
$a = array(1,2,3);
9+
$b = array(1);
10+
$c = array(3);
11+
$d = array_diff($a, $b, $c);
12+
echo implode(',', $d) . PHP_EOL; // expecting '2'
13+
?>
14+
--EXPECT--
15+
2
16+
--CLEAN--
17+
<?php
18+
unset($a, $b, $c, $d);

0 commit comments

Comments
 (0)