[RFC] Path to Saner Increment/Decrement operators (#10358)

* Add behavioural tests for incdec operators

* Add support to ++/-- for objects castable to _IS_NUMBER

* Add str_increment() function

* Add str_decrement() function

RFC: https://wiki.php.net/rfc/saner-inc-dec-operators

Co-authored-by: Ilija Tovilo <[email protected]>
Co-authored-by: Arnaud Le Blanc <[email protected]>

Author: 3 people

UPGRADING

@@ -118,6 +118,14 @@ PHP 8.3 UPGRADE NOTES
 4. Deprecated Functionality
 ========================================
 
+- Core
+  . Using the ++ operator on empty, non-numeric, or non-alphanumeric strings
+    is now deprecated. Moreover, incrementing non-numeric strings is soft
+    deprecated and the new str_increment() function should be used instead.
+    RFC: https://wiki.php.net/rfc/saner-inc-dec-operators
+  . Using the -- operator on empty or non-numeric strings is now deprecated.
+    RFC: https://wiki.php.net/rfc/saner-inc-dec-operators
+
 - Intl
   . The U_MULTIPLE_DECIMAL_SEP*E*RATORS constant had been deprecated, using
     the U_MULTIPLE_DECIMAL_SEP*A*RATORS instead is recommended.
@@ -303,6 +311,10 @@ PHP 8.3 UPGRADE NOTES
 - Sockets:
   . Added socket_atmark to checks if the socket is OOB marked.
 
+- Standard:
+  . Added the str_increment() and str_decrement() functions.
+    RFC: https://wiki.php.net/rfc/saner-inc-dec-operators
+
 - Zip:
   . Added ZipArchive::setArchiveFlag and ZipArchive::getArchiveFlag methods.
 
@@ -324,6 +336,13 @@ PHP 8.3 UPGRADE NOTES
     not reachable except by iterating over the WeakMap (reachability via
     iteration is considered weak). Previously, such entries would never be
     automatically removed. (See GH-10932.)
+  . The ++ and -- operators now emit warnings when attempting to increment
+    values of type bool as this will change in the next major version.
+    A warning is emitted when trying to decrement values of type null, as
+    this will change in the next major version.
+    Internal objects that implement an _IS_NUMBER cast but not a do_operator
+    handler that overrides addition and substraction now can be incremented
+    and decrement as if one would do $o += 1 or $o -= 1
 
 - DOM:
   . The DOM lifetime mechanism has been reworked such that implicitly removed

Zend/Optimizer/sccp.c

@@ -664,9 +664,14 @@ static inline zend_result ct_eval_assign_obj(zval *result, zval *value, const zv
 }
 
 static inline zend_result ct_eval_incdec(zval *result, uint8_t opcode, zval *op1) {
+	/* As of PHP 8.3 with the warning/deprecation notices any type other than int/double/null will emit a diagnostic
 	if (Z_TYPE_P(op1) == IS_ARRAY || IS_PARTIAL_ARRAY(op1)) {
 		return FAILURE;
 	}
+	*/
+	if (Z_TYPE_P(op1) != IS_LONG && Z_TYPE_P(op1) != IS_DOUBLE && Z_TYPE_P(op1) != IS_NULL) {
+		return FAILURE;
+	}
 
 	ZVAL_COPY(result, op1);
 	if (opcode == ZEND_PRE_INC
@@ -675,6 +680,11 @@ static inline zend_result ct_eval_incdec(zval *result, uint8_t opcode, zval *op1
 			|| opcode == ZEND_POST_INC_OBJ) {
 		increment_function(result);
 	} else {
+		/* Decrement on null emits a deprecation notice */
+		if (Z_TYPE_P(op1) == IS_NULL) {
+			zval_ptr_dtor(result);
+			return FAILURE;
+		}
 		decrement_function(result);
 	}
 	return SUCCESS;
@@ -1375,21 +1385,22 @@ static void sccp_visit_instr(scdf_ctx *scdf, zend_op *opline, zend_ssa_op *ssa_o
 						&& ctx->scdf.ssa->vars[ssa_op->op1_def].escape_state == ESCAPE_STATE_NO_ESCAPE) {
 					zval tmp1, tmp2;
 
-					if (ct_eval_fetch_obj(&tmp1, op1, op2) == SUCCESS
-							&& ct_eval_incdec(&tmp2, opline->opcode, &tmp1) == SUCCESS) {
-
-						dup_partial_object(&zv, op1);
-						ct_eval_assign_obj(&zv, &tmp2, op2);
-						if (opline->opcode == ZEND_PRE_INC_OBJ || opline->opcode == ZEND_PRE_DEC_OBJ) {
-							SET_RESULT(result, &tmp2);
-						} else {
-							SET_RESULT(result, &tmp1);
+					if (ct_eval_fetch_obj(&tmp1, op1, op2) == SUCCESS) {
+						if (ct_eval_incdec(&tmp2, opline->opcode, &tmp1) == SUCCESS) {
+							dup_partial_object(&zv, op1);
+							ct_eval_assign_obj(&zv, &tmp2, op2);
+							if (opline->opcode == ZEND_PRE_INC_OBJ || opline->opcode == ZEND_PRE_DEC_OBJ) {
+								SET_RESULT(result, &tmp2);
+							} else {
+								SET_RESULT(result, &tmp1);
+							}
+							zval_ptr_dtor_nogc(&tmp1);
+							zval_ptr_dtor_nogc(&tmp2);
+							SET_RESULT(op1, &zv);
+							zval_ptr_dtor_nogc(&zv);
+							break;
 						}
 						zval_ptr_dtor_nogc(&tmp1);
-						zval_ptr_dtor_nogc(&tmp2);
-						SET_RESULT(op1, &zv);
-						zval_ptr_dtor_nogc(&zv);
-						break;
 					}
 				}
 			}
@@ -2109,7 +2120,7 @@ static int try_remove_definition(sccp_ctx *ctx, int var_num, zend_ssa_var *var,
 							break;
 						default:
 							break;
-					}	
+					}
 				}
 				/* we cannot remove instruction that defines other variables */
 				return 0;

Zend/Optimizer/zend_inference.c

@@ -4681,9 +4681,11 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
 			return (t1 & (MAY_BE_NULL|MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_DOUBLE|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
 		case ZEND_PRE_INC:
 		case ZEND_POST_INC:
+			return (t1 & (MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
+		/* null emits a warning as it has no effect compared to ++ which converts the value to 1 */
 		case ZEND_PRE_DEC:
 		case ZEND_POST_DEC:
-			return (t1 & (MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
+			return (t1 & (MAY_BE_NULL|MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_STRING|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
 		case ZEND_BOOL_NOT:
 		case ZEND_JMPZ:
 		case ZEND_JMPNZ:

Zend/tests/decrement_001.phpt

@@ -0,0 +1,25 @@
+--TEST--
+Decrementing min int values 32bit
+--SKIPIF--
+<?php if (PHP_INT_SIZE != 4) die("skip this test is for 32bit platform only"); ?>
+--INI--
+precision=14
+--FILE--
+<?php
+
+$values = [
+    -PHP_INT_MAX-1,
+    (string)(-PHP_INT_MAX-1),
+];
+
+foreach ($values as $var) {
+    $var--;
+    var_dump($var);
+}
+
+echo "Done\n";
+?>
+--EXPECT--
+float(-2147483649)
+float(-2147483649)
+Done

Zend/tests/decrement_001_64bit.phpt

@@ -0,0 +1,25 @@
+--TEST--
+Decrementing min int values 64bit
+--SKIPIF--
+<?php if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platform only"); ?>
+--INI--
+precision=14
+--FILE--
+<?php
+
+$values = [
+    -PHP_INT_MAX-1,
+    (string)(-PHP_INT_MAX-1),
+];
+
+foreach ($values as $var) {
+    $var--;
+    var_dump($var);
+}
+
+echo "Done\n";
+?>
+--EXPECT--
+float(-9.223372036854776E+18)
+float(-9.223372036854776E+18)
+Done

Zend/tests/in-de-crement/decrement_001.phpt

@@ -0,0 +1,19 @@
+--TEST--
+Error handler can change type of operand of --
+--FILE--
+<?php
+
+set_error_handler(function () {
+    global $x;
+    $x = 1;
+});
+
+$x = '';
+$x--;
+var_dump($x);
+
+?>
+DONE
+--EXPECT--
+int(-1)
+DONE