RFC: Marking return values as important (#[\NoDiscard])

NEWS

@@ -38,8 +38,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-18033 (NULL-ptr dereference when using register_tick_function
     in destructor). (nielsdos)
   . Fixed bug GH-18026 (Improve "expecting token" error for ampersand). (ilutov)
+  . Added the #[\NoDiscard] attribute to indicate that a function's return
+    value is important and should be consumed. (timwolla, Volker Dusch)
   . Added the (void) cast to indicate that not using a value is intentional.
-    (timwolla)
+    (timwolla, Volker Dusch)
   . Added get_error_handler(), get_exception_handler() functions. (Arnaud)
   . Fixed bug GH-15753 and GH-16198 (Bind traits before parent class). (ilutov)
 

UPGRADING

@@ -114,8 +114,13 @@ PHP 8.5 UPGRADE NOTES
   . Fatal Errors (such as an exceeded maximum execution time) now include a
     backtrace.
     RFC: https://wiki.php.net/rfc/error_backtraces_v2
-  . Added the (void) to indicate that not using a value is intentional. The
-    (void) cast does nothing by itself.
+  . Added the #[\NoDiscard] attribute to indicate that a function's return
+    value is important and should be consumed.
+    RFC: https://wiki.php.net/rfc/marking_return_value_as_important
+  . Added the (void) cast to indicate that not using a value is intentional.
+    The (void) cast has no effect on the program's execution by itself, but
+    it can be used to suppress warnings emitted by #[\NoDiscard] and possibly
+    also diagnostics emitted by external IDEs or static analysis tools.
     RFC: https://wiki.php.net/rfc/marking_return_value_as_important
 
 - Curl:

Zend/Optimizer/optimize_func_calls.c

@@ -78,8 +78,10 @@ static void zend_delete_call_instructions(zend_op_array *op_array, zend_op *opli
 
 static void zend_try_inline_call(zend_op_array *op_array, zend_op *fcall, zend_op *opline, zend_function *func)
 {
+	const uint32_t no_discard = RETURN_VALUE_USED(opline) ? 0 : ZEND_ACC_NODISCARD;
+
 	if (func->type == ZEND_USER_FUNCTION
-	 && !(func->op_array.fn_flags & (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS|ZEND_ACC_DEPRECATED))
+	 && !(func->op_array.fn_flags & (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS|ZEND_ACC_DEPRECATED|no_discard))
 		/* TODO: function copied from trait may be inconsistent ??? */
 	 && !(func->op_array.fn_flags & (ZEND_ACC_TRAIT_CLONE))
 	 && fcall->extended_value >= func->op_array.required_num_args
@@ -202,18 +204,12 @@ void zend_optimize_func_calls(zend_op_array *op_array, zend_optimizer_ctx *ctx)
 						fcall->op1.num = zend_vm_calc_used_stack(fcall->extended_value, call_stack[call].func);
 						literal_dtor(&ZEND_OP2_LITERAL(fcall));
 						fcall->op2.constant = fcall->op2.constant + 1;
-						if (opline->opcode != ZEND_CALLABLE_CONVERT) {
-							opline->opcode = zend_get_call_op(fcall, call_stack[call].func);
-						}
 					} else if (fcall->opcode == ZEND_INIT_NS_FCALL_BY_NAME) {
 						fcall->opcode = ZEND_INIT_FCALL;
 						fcall->op1.num = zend_vm_calc_used_stack(fcall->extended_value, call_stack[call].func);
 						literal_dtor(&op_array->literals[fcall->op2.constant]);
 						literal_dtor(&op_array->literals[fcall->op2.constant + 2]);
 						fcall->op2.constant = fcall->op2.constant + 1;
-						if (opline->opcode != ZEND_CALLABLE_CONVERT) {
-							opline->opcode = zend_get_call_op(fcall, call_stack[call].func);
-						}
 					} else if (fcall->opcode == ZEND_INIT_STATIC_METHOD_CALL
 							|| fcall->opcode == ZEND_INIT_METHOD_CALL
 							|| fcall->opcode == ZEND_INIT_PARENT_PROPERTY_HOOK_CALL
@@ -223,6 +219,16 @@ void zend_optimize_func_calls(zend_op_array *op_array, zend_optimizer_ctx *ctx)
 						ZEND_UNREACHABLE();
 					}
 
+					/* If the INIT opcode changed the DO opcode can also change to
+					 * a more optimized one.
+					 *
+					 * At this point we also know whether or not the result of
+					 * the DO opcode is used, allowing to optimize calls to
+					 * ZEND_ACC_NODISCARD functions. */
+					if (opline->opcode != ZEND_CALLABLE_CONVERT) {
+						opline->opcode = zend_get_call_op(fcall, call_stack[call].func, !RESULT_UNUSED(opline));
+					}
+
 					if ((ZEND_OPTIMIZER_PASS_16 & ctx->optimization_level)
 							&& call_stack[call].try_inline
 							&& opline->opcode != ZEND_CALLABLE_CONVERT) {

Zend/tests/attributes/nodiscard/001.phpt

@@ -0,0 +1,86 @@
+--TEST--
+#[\NoDiscard]: Basic test.
+--FILE--
+<?php
+
+#[\NoDiscard]
+function test(): int {
+	return 0;
+}
+
+#[\NoDiscard("this is important")]
+function test2(): int {
+	return 0;
+}
+
+#[\NoDiscard]
+function test3(...$args): int {
+	return 0;
+}
+
+class Clazz {
+	#[\NoDiscard]
+	function test(): int {
+		return 0;
+	}
+
+	#[\NoDiscard("this is important")]
+	function test2(): int {
+		return 0;
+	}
+
+	#[\NoDiscard]
+	static function test3(): int {
+		return 0;
+	}
+}
+
+$closure = #[\NoDiscard] function(): int {
+	return 0;
+};
+
+$closure2 = #[\NoDiscard] function(): int {
+	return 0;
+};
+
+test();
+test2();
+test3(1, 2, named: 3);
+call_user_func("test");
+$fcc = test(...);
+$fcc();
+
+$cls = new Clazz();
+$cls->test();
+$cls->test2();
+Clazz::test3();
+
+call_user_func([$cls, "test"]);
+
+$closure();
+
+$closure2();
+
+?>
+--EXPECTF--
+Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of function test2() should either be used or intentionally ignored by casting it as (void), this is important in %s on line %d
+
+Warning: The return value of function test3() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of method Clazz::test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of method Clazz::test2() should either be used or intentionally ignored by casting it as (void), this is important in %s on line %d
+
+Warning: The return value of method Clazz::test3() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of method Clazz::test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of function {closure:%s:%d}() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+
+Warning: The return value of function {closure:%s:%d}() should either be used or intentionally ignored by casting it as (void) in %s on line %d

Zend/tests/attributes/nodiscard/002.phpt

@@ -0,0 +1,44 @@
+--TEST--
+#[\NoDiscard]: __call(), __callStatic(), and __invoke().
+--FILE--
+<?php
+
+class Clazz {
+	#[\NoDiscard]
+	public function __call(string $name, array $args): int {
+		echo "__call({$name})", PHP_EOL;
+
+		return strlen($name);
+	}
+
+	#[\NoDiscard]
+	public static function __callStatic(string $name, array $args): int {
+		echo "__callStatic({$name})", PHP_EOL;
+
+		return strlen($name);
+	}
+
+	#[\NoDiscard]
+	public function __invoke(string $param): int {
+		echo "__invoke({$param})", PHP_EOL;
+
+		return strlen($param);
+	}
+}
+
+$cls = new Clazz();
+$cls->test();
+Clazz::test();
+$cls('foo');
+
+?>
+--EXPECTF--
+Warning: The return value of method Clazz::test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+__call(test)
+
+Warning: The return value of method Clazz::test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+__callStatic(test)
+
+Warning: The return value of method Clazz::__invoke() should either be used or intentionally ignored by casting it as (void) in %s on line %d
+__invoke(foo)
+

Zend/tests/attributes/nodiscard/003.phpt

@@ -0,0 +1,22 @@
+--TEST--
+#[\NoDiscard]: Taken from trait.
+--FILE--
+<?php
+
+trait T {
+	#[\NoDiscard]
+	function test(): int {
+		return 0;
+	}
+}
+
+class Clazz {
+	use T;
+}
+
+$cls = new Clazz();
+$cls->test();
+
+?>
+--EXPECTF--
+Warning: The return value of method Clazz::test() should either be used or intentionally ignored by casting it as (void) in %s on line %d

Zend/tests/attributes/nodiscard/005.phpt

@@ -0,0 +1,17 @@
+--TEST--
+#[\NoDiscard]: Native function and method.
+--FILE--
+<?php
+
+$f = tmpfile();
+flock($f, LOCK_SH | LOCK_NB);
+fclose($f);
+
+$date = new DateTimeImmutable('now');
+$date->setTimestamp(0);
+
+?>
+--EXPECTF--
+Warning: The return value of function flock() should either be used or intentionally ignored by casting it as (void), as locking the stream might have failed in %s on line %d
+
+Warning: The return value of method DateTimeImmutable::setTimestamp() should either be used or intentionally ignored by casting it as (void), as DateTimeImmutable::setTimestamp() does not modify the object itself in %s on line %d

Zend/tests/attributes/nodiscard/006.phpt

@@ -0,0 +1,20 @@
+--TEST--
+#[\NoDiscard]: execute_ex overwritten
+--EXTENSIONS--
+zend_test
+--INI--
+zend_test.replace_zend_execute_ex=1
+opcache.jit=disable
+--FILE--
+<?php
+
+#[\NoDiscard]
+function test(): int {
+	return 0;
+}
+
+test();
+
+?>
+--EXPECTF--
+Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d

Zend/tests/attributes/nodiscard/007.phpt

@@ -0,0 +1,21 @@
+--TEST--
+#[\NoDiscard]: execute_internal overwritten
+--EXTENSIONS--
+zend_test
+--INI--
+zend_test.observer.execute_internal=1
+--FILE--
+<?php
+
+$f = tmpfile();
+flock($f, LOCK_SH | LOCK_NB);
+fclose($f);
+
+?>
+--EXPECTF--
+<!-- internal enter tmpfile() -->
+<!-- internal enter NoDiscard::__construct() -->
+
+Warning: The return value of function flock() should either be used or intentionally ignored by casting it as (void), as locking the stream might have failed in %s on line %d
+<!-- internal enter flock() -->
+<!-- internal enter fclose() -->

Zend/tests/attributes/nodiscard/008.phpt

@@ -0,0 +1,18 @@
+--TEST--
+#[\NoDiscard]: Combining with #[\Deprecated].
+--FILE--
+<?php
+
+#[\NoDiscard]
+#[\Deprecated]
+function test(): int {
+	return 0;
+}
+
+test();
+
+?>
+--EXPECTF--
+Deprecated: Function test() is deprecated in %s on line %d
+
+Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d

Zend/tests/attributes/nodiscard/009.phpt

@@ -0,0 +1,14 @@
+--TEST--
+#[\NoDiscard]: Combining with #[\Deprecated] (Internal).
+--EXTENSIONS--
+zend_test
+--FILE--
+<?php
+
+zend_test_deprecated_nodiscard();
+
+?>
+--EXPECTF--
+Deprecated: Function zend_test_deprecated_nodiscard() is deprecated, custom message in %s on line %d
+
+Warning: The return value of function zend_test_deprecated_nodiscard() should either be used or intentionally ignored by casting it as (void), custom message 2 in %s on line %d