Skip to content

Commit 3b06618

Browse files
GirgiasGirgias
authored
RFC: Saner array_(sum|product)() (#10161)
RFC: https://wiki.php.net/rfc/saner-array-sum-product Moreover, the internal fast_add_function() function was removed.
1 parent 88b30e0 commit 3b06618

22 files changed

+476
-101
lines changed

‎UPGRADING.INTERNALS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ PHP 8.3 INTERNALS UPGRADE NOTES
3737
* The return types of the following functions have been changed from
3838
`bool` to `zend_result`:
3939
- zend_fiber_init_context()
40+
* The fast_add_function() has been removed, use add_function() that will
41+
call the static inline add_function_fast() instead.
4042

4143
========================
4244
2. Build system changes

‎Zend/zend_operators.h‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -707,28 +707,6 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL
707707
#endif
708708
}
709709

710-
static zend_always_inline zend_result fast_add_function(zval *result, zval *op1, zval *op2)
711-
{
712-
if (EXPECTED(Z_TYPE_P(op1) == IS_LONG)) {
713-
if (EXPECTED(Z_TYPE_P(op2) == IS_LONG)) {
714-
fast_long_add_function(result, op1, op2);
715-
return SUCCESS;
716-
} else if (EXPECTED(Z_TYPE_P(op2) == IS_DOUBLE)) {
717-
ZVAL_DOUBLE(result, ((double)Z_LVAL_P(op1)) + Z_DVAL_P(op2));
718-
return SUCCESS;
719-
}
720-
} else if (EXPECTED(Z_TYPE_P(op1) == IS_DOUBLE)) {
721-
if (EXPECTED(Z_TYPE_P(op2) == IS_DOUBLE)) {
722-
ZVAL_DOUBLE(result, Z_DVAL_P(op1) + Z_DVAL_P(op2));
723-
return SUCCESS;
724-
} else if (EXPECTED(Z_TYPE_P(op2) == IS_LONG)) {
725-
ZVAL_DOUBLE(result, Z_DVAL_P(op1) + ((double)Z_LVAL_P(op2)));
726-
return SUCCESS;
727-
}
728-
}
729-
return add_function(result, op1, op2);
730-
}
731-
732710
static zend_always_inline void fast_long_sub_function(zval *result, zval *op1, zval *op2)
733711
{
734712
#if ZEND_USE_ASM_ARITHMETIC && defined(__i386__) && !(4 == __GNUC__ && 8 == __GNUC_MINOR__)

‎ext/standard/array.c‎

Lines changed: 49 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5915,65 +5915,70 @@ PHP_FUNCTION(array_rand)
59155915
}
59165916
/* }}} */
59175917

5918-
/* {{{ Returns the sum of the array entries */
5919-
PHP_FUNCTION(array_sum)
5918+
/* Wrapper for array_sum and array_product */
5919+
static void php_array_binop(INTERNAL_FUNCTION_PARAMETERS, const char *op_name, binary_op_type op, zend_long initial)
59205920
{
5921-
zval *input,
5922-
*entry,
5923-
entry_n;
5921+
HashTable *input;
5922+
zval *entry;
59245923

59255924
ZEND_PARSE_PARAMETERS_START(1, 1)
5926-
Z_PARAM_ARRAY(input)
5925+
Z_PARAM_ARRAY_HT(input)
59275926
ZEND_PARSE_PARAMETERS_END();
59285927

5929-
ZVAL_LONG(return_value, 0);
5928+
if (zend_hash_num_elements(input) == 0) {
5929+
RETURN_LONG(initial);
5930+
}
5931+
5932+
ZVAL_LONG(return_value, initial);
5933+
ZEND_HASH_FOREACH_VAL(input, entry) {
5934+
/* For objects we try to cast them to a numeric type */
5935+
if (Z_TYPE_P(entry) == IS_OBJECT) {
5936+
zval dst;
5937+
zend_result status = Z_OBJ_HT_P(entry)->cast_object(Z_OBJ_P(entry), &dst, _IS_NUMBER);
59305938

5931-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(input), entry) {
5932-
if (Z_TYPE_P(entry) == IS_ARRAY || Z_TYPE_P(entry) == IS_OBJECT) {
5939+
/* Do not type error for BC */
5940+
if (status == FAILURE || (Z_TYPE(dst) != IS_LONG && Z_TYPE(dst) != IS_DOUBLE)) {
5941+
php_error_docref(NULL, E_WARNING, "%s is not supported on type %s",
5942+
op_name, zend_zval_type_name(entry));
5943+
continue;
5944+
}
5945+
op(return_value, return_value, &dst);
59335946
continue;
59345947
}
5935-
ZVAL_COPY(&entry_n, entry);
5936-
convert_scalar_to_number(&entry_n);
5937-
fast_add_function(return_value, return_value, &entry_n);
5948+
5949+
zend_result status = op(return_value, return_value, entry);
5950+
if (status == FAILURE) {
5951+
ZEND_ASSERT(EG(exception));
5952+
zend_clear_exception();
5953+
/* BC resources: previously resources were cast to int */
5954+
if (Z_TYPE_P(entry) == IS_RESOURCE) {
5955+
zval tmp;
5956+
ZVAL_LONG(&tmp, Z_RES_HANDLE_P(entry));
5957+
op(return_value, return_value, &tmp);
5958+
}
5959+
/* BC non numeric strings: previously were cast to 0 */
5960+
else if (Z_TYPE_P(entry) == IS_STRING) {
5961+
zval tmp;
5962+
ZVAL_LONG(&tmp, 0);
5963+
op(return_value, return_value, &tmp);
5964+
}
5965+
php_error_docref(NULL, E_WARNING, "%s is not supported on type %s",
5966+
op_name, zend_zval_type_name(entry));
5967+
}
59385968
} ZEND_HASH_FOREACH_END();
59395969
}
5970+
5971+
/* {{{ Returns the sum of the array entries */
5972+
PHP_FUNCTION(array_sum)
5973+
{
5974+
php_array_binop(INTERNAL_FUNCTION_PARAM_PASSTHRU, "Addition", add_function, 0);
5975+
}
59405976
/* }}} */
59415977

59425978
/* {{{ Returns the product of the array entries */
59435979
PHP_FUNCTION(array_product)
59445980
{
5945-
zval *input,
5946-
*entry,
5947-
entry_n;
5948-
double dval;
5949-
5950-
ZEND_PARSE_PARAMETERS_START(1, 1)
5951-
Z_PARAM_ARRAY(input)
5952-
ZEND_PARSE_PARAMETERS_END();
5953-
5954-
ZVAL_LONG(return_value, 1);
5955-
if (!zend_hash_num_elements(Z_ARRVAL_P(input))) {
5956-
return;
5957-
}
5958-
5959-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(input), entry) {
5960-
if (Z_TYPE_P(entry) == IS_ARRAY || Z_TYPE_P(entry) == IS_OBJECT) {
5961-
continue;
5962-
}
5963-
ZVAL_COPY(&entry_n, entry);
5964-
convert_scalar_to_number(&entry_n);
5965-
5966-
if (Z_TYPE(entry_n) == IS_LONG && Z_TYPE_P(return_value) == IS_LONG) {
5967-
dval = (double)Z_LVAL_P(return_value) * (double)Z_LVAL(entry_n);
5968-
if ( (double)ZEND_LONG_MIN <= dval && dval <= (double)ZEND_LONG_MAX ) {
5969-
Z_LVAL_P(return_value) *= Z_LVAL(entry_n);
5970-
continue;
5971-
}
5972-
}
5973-
convert_to_double(return_value);
5974-
convert_to_double(&entry_n);
5975-
Z_DVAL_P(return_value) *= Z_DVAL(entry_n);
5976-
} ZEND_HASH_FOREACH_END();
5981+
php_array_binop(INTERNAL_FUNCTION_PARAM_PASSTHRU, "Multiplication", mul_function, 1);
59775982
}
59785983
/* }}} */
59795984

‎ext/standard/tests/array/003.phpt‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,27 @@ require(__DIR__ . '/data.inc');
88

99
function cmp ($a, $b) {
1010
is_array ($a)
11-
and $a = array_sum ($a);
11+
and $a = count($a);
1212
is_array ($b)
13-
and $b = array_sum ($b);
13+
and $b = count($b);
1414
return strcmp ($a, $b);
1515
}
1616

17-
echo " -- Testing uasort() -- \n";
17+
echo "-- Testing uasort() --\n";
1818
uasort ($data, 'cmp');
1919
var_dump ($data);
2020

2121

22-
echo "\n -- Testing uksort() -- \n";
22+
echo "\n-- Testing uksort() --\n";
2323
uksort ($data, 'cmp');
2424
var_dump ($data);
2525

26-
echo "\n -- Testing usort() -- \n";
26+
echo "\n-- Testing usort() --\n";
2727
usort ($data, 'cmp');
2828
var_dump ($data);
2929
?>
3030
--EXPECT--
31-
-- Testing uasort() --
31+
-- Testing uasort() --
3232
array(8) {
3333
[16777216]=>
3434
float(-0.3333333333333333)
@@ -53,7 +53,7 @@ array(8) {
5353
string(4) "test"
5454
}
5555

56-
-- Testing uksort() --
56+
-- Testing uksort() --
5757
array(8) {
5858
[-1000]=>
5959
array(2) {
@@ -78,7 +78,7 @@ array(8) {
7878
int(27)
7979
}
8080

81-
-- Testing usort() --
81+
-- Testing usort() --
8282
array(8) {
8383
[0]=>
8484
float(-0.3333333333333333)

‎ext/standard/tests/array/array_product_empty_array.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Test array_product() function with empty array
3+
--FILE--
4+
<?php
5+
$input = [];
6+
7+
echo "array_product() version:\n";
8+
var_dump(array_product($input));
9+
10+
echo "array_reduce() version:\n";
11+
var_dump(array_reduce($input, fn($carry, $value) => $carry * $value, 1));
12+
?>
13+
--EXPECT--
14+
array_product() version:
15+
int(1)
16+
array_reduce() version:
17+
int(1)

‎ext/standard/tests/array/array_product_objects_operation_no_cast.phpt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
Test array_product() function with objects that implement addition but not castable to numeric type
3+
--EXTENSIONS--
4+
zend_test
5+
--FILE--
6+
<?php
7+
$input = [new DoOperationNoCast(25), new DoOperationNoCast(6), new DoOperationNoCast(10)];
8+
9+
echo "array_product() version:\n";
10+
var_dump(array_product($input));
11+
12+
echo "array_reduce() version:\n";
13+
var_dump(array_reduce($input, fn($carry, $value) => $carry * $value, 1));
14+
?>
15+
--EXPECTF--
16+
array_product() version:
17+
18+
Warning: array_product(): Multiplication is not supported on type DoOperationNoCast in %s on line %d
19+
20+
Warning: array_product(): Multiplication is not supported on type DoOperationNoCast in %s on line %d
21+
22+
Warning: array_product(): Multiplication is not supported on type DoOperationNoCast in %s on line %d
23+
int(1)
24+
array_reduce() version:
25+
object(DoOperationNoCast)#5 (1) {
26+
["val":"DoOperationNoCast":private]=>
27+
int(1500)
28+
}

‎ext/standard/tests/array/array_product_variation1.phpt‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,18 @@ echo "*** Testing array_product() : variation - using non numeric values ***\n";
77
class A {
88
static function help() { echo "hello\n"; }
99
}
10-
$fp = fopen(__FILE__, "r");
1110

1211
$types = array("boolean (true)" => true, "boolean (false)" => false,
1312
"string" => "hello", "numeric string" => "12",
14-
"resource" => $fp, "object" => new A(), "null" => null,
13+
"resource" => STDERR, "object" => new A(), "null" => null,
1514
"array" => array(3,2));
1615

1716
foreach ($types as $desc => $type) {
18-
echo $desc . "\n";
19-
var_dump(array_product(array($type)));
17+
echo $desc, "\n";
18+
var_dump(array_product([1, $type]));
2019
echo "\n";
2120
}
2221

23-
fclose($fp);
2422
?>
2523
--EXPECTF--
2624
*** Testing array_product() : variation - using non numeric values ***
@@ -31,20 +29,28 @@ boolean (false)
3129
int(0)
3230

3331
string
32+
33+
Warning: array_product(): Multiplication is not supported on type string in %s on line %d
3434
int(0)
3535

3636
numeric string
3737
int(12)
3838

3939
resource
40-
int(%d)
40+
41+
Warning: array_product(): Multiplication is not supported on type resource in %s on line %d
42+
int(3)
4143

4244
object
45+
46+
Warning: array_product(): Multiplication is not supported on type A in %s on line %d
4347
int(1)
4448

4549
null
4650
int(0)
4751

4852
array
53+
54+
Warning: array_product(): Multiplication is not supported on type array in %s on line %d
4955
int(1)
5056

‎ext/standard/tests/array/array_product_variation5.phpt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
Test array_product() function: resources in array
3+
--FILE--
4+
<?php
5+
$input = [10, STDERR /* Should get casted to 3 as an integer */];
6+
7+
echo "array_product() version:\n";
8+
var_dump(array_product($input));
9+
10+
echo "array_reduce() version:\n";
11+
try {
12+
var_dump(array_reduce($input, fn($carry, $value) => $carry * $value, 1));
13+
} catch (TypeError $e) {
14+
echo $e->getMessage();
15+
}
16+
?>
17+
--EXPECTF--
18+
array_product() version:
19+
20+
Warning: array_product(): Multiplication is not supported on type resource in %s on line %d
21+
int(30)
22+
array_reduce() version:
23+
Unsupported operand types: int * resource

‎ext/standard/tests/array/array_product_variation6.phpt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
Test array_product() function with objects castable to numeric type
3+
--EXTENSIONS--
4+
gmp
5+
--FILE--
6+
<?php
7+
$input = [gmp_init(25), gmp_init(6)];
8+
9+
echo "array_product() version:\n";
10+
var_dump(array_product($input));
11+
12+
echo "array_reduce() version:\n";
13+
var_dump(array_reduce($input, fn($carry, $value) => $carry * $value, 1));
14+
?>
15+
--EXPECT--
16+
array_product() version:
17+
int(150)
18+
array_reduce() version:
19+
object(GMP)#5 (1) {
20+
["num"]=>
21+
string(3) "150"
22+
}

‎ext/standard/tests/array/array_sum_empty_array.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Test array_sum() function with empty array
3+
--FILE--
4+
<?php
5+
$input = [];
6+
7+
echo "array_sum() version:\n";
8+
var_dump(array_sum($input));
9+
10+
echo "array_reduce() version:\n";
11+
var_dump(array_reduce($input, fn($carry, $value) => $carry + $value, 0));
12+
?>
13+
--EXPECT--
14+
array_sum() version:
15+
int(0)
16+
array_reduce() version:
17+
int(0)

0 commit comments

Comments
 (0)