Skip to content

Commit 5544be7

Browse files
TimWollaedorian
TimWolla
and
edorian
authored
RFC: Marking return values as important (#[\NoDiscard]) (#17599)
RFC: https://wiki.php.net/rfc/marking_return_value_as_important Co-authored-by: Volker Dusch <volker@tideways-gmbh.com>
1 parent f11c22a commit 5544be7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1261
-68
lines changed

‎NEWS

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,10 @@ PHP NEWS
3838
. Fixed bug GH-18033 (NULL-ptr dereference when using register_tick_function
3939
in destructor). (nielsdos)
4040
. Fixed bug GH-18026 (Improve "expecting token" error for ampersand). (ilutov)
41+
. Added the #[\NoDiscard] attribute to indicate that a function's return
42+
value is important and should be consumed. (timwolla, Volker Dusch)
4143
. Added the (void) cast to indicate that not using a value is intentional.
42-
(timwolla)
44+
(timwolla, Volker Dusch)
4345
. Added get_error_handler(), get_exception_handler() functions. (Arnaud)
4446
. Fixed bug GH-15753 and GH-16198 (Bind traits before parent class). (ilutov)
4547

‎UPGRADING

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,13 @@ PHP 8.5 UPGRADE NOTES
114114
. Fatal Errors (such as an exceeded maximum execution time) now include a
115115
backtrace.
116116
RFC: https://wiki.php.net/rfc/error_backtraces_v2
117-
. Added the (void) to indicate that not using a value is intentional. The
118-
(void) cast does nothing by itself.
117+
. Added the #[\NoDiscard] attribute to indicate that a function's return
118+
value is important and should be consumed.
119+
RFC: https://wiki.php.net/rfc/marking_return_value_as_important
120+
. Added the (void) cast to indicate that not using a value is intentional.
121+
The (void) cast has no effect on the program's execution by itself, but
122+
it can be used to suppress warnings emitted by #[\NoDiscard] and possibly
123+
also diagnostics emitted by external IDEs or static analysis tools.
119124
RFC: https://wiki.php.net/rfc/marking_return_value_as_important
120125

121126
- Curl:

‎Zend/Optimizer/optimize_func_calls.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,10 @@ static void zend_delete_call_instructions(zend_op_array *op_array, zend_op *opli
7878

7979
static void zend_try_inline_call(zend_op_array *op_array, zend_op *fcall, zend_op *opline, zend_function *func)
8080
{
81+
const uint32_t no_discard = RETURN_VALUE_USED(opline) ? 0 : ZEND_ACC_NODISCARD;
82+
8183
if (func->type == ZEND_USER_FUNCTION
82-
&& !(func->op_array.fn_flags & (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS|ZEND_ACC_DEPRECATED))
84+
&& !(func->op_array.fn_flags & (ZEND_ACC_ABSTRACT|ZEND_ACC_HAS_TYPE_HINTS|ZEND_ACC_DEPRECATED|no_discard))
8385
/* TODO: function copied from trait may be inconsistent ??? */
8486
&& !(func->op_array.fn_flags & (ZEND_ACC_TRAIT_CLONE))
8587
&& 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)
202204
fcall->op1.num = zend_vm_calc_used_stack(fcall->extended_value, call_stack[call].func);
203205
literal_dtor(&ZEND_OP2_LITERAL(fcall));
204206
fcall->op2.constant = fcall->op2.constant + 1;
205-
if (opline->opcode != ZEND_CALLABLE_CONVERT) {
206-
opline->opcode = zend_get_call_op(fcall, call_stack[call].func);
207-
}
208207
} else if (fcall->opcode == ZEND_INIT_NS_FCALL_BY_NAME) {
209208
fcall->opcode = ZEND_INIT_FCALL;
210209
fcall->op1.num = zend_vm_calc_used_stack(fcall->extended_value, call_stack[call].func);
211210
literal_dtor(&op_array->literals[fcall->op2.constant]);
212211
literal_dtor(&op_array->literals[fcall->op2.constant + 2]);
213212
fcall->op2.constant = fcall->op2.constant + 1;
214-
if (opline->opcode != ZEND_CALLABLE_CONVERT) {
215-
opline->opcode = zend_get_call_op(fcall, call_stack[call].func);
216-
}
217213
} else if (fcall->opcode == ZEND_INIT_STATIC_METHOD_CALL
218214
|| fcall->opcode == ZEND_INIT_METHOD_CALL
219215
|| 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)
223219
ZEND_UNREACHABLE();
224220
}
225221

222+
/* If the INIT opcode changed the DO opcode can also change to
223+
* a more optimized one.
224+
*
225+
* At this point we also know whether or not the result of
226+
* the DO opcode is used, allowing to optimize calls to
227+
* ZEND_ACC_NODISCARD functions. */
228+
if (opline->opcode != ZEND_CALLABLE_CONVERT) {
229+
opline->opcode = zend_get_call_op(fcall, call_stack[call].func, !RESULT_UNUSED(opline));
230+
}
231+
226232
if ((ZEND_OPTIMIZER_PASS_16 & ctx->optimization_level)
227233
&& call_stack[call].try_inline
228234
&& opline->opcode != ZEND_CALLABLE_CONVERT) {

‎Zend/tests/attributes/nodiscard/001.phpt

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
--TEST--
2+
#[\NoDiscard]: Basic test.
3+
--FILE--
4+
<?php
5+
6+
#[\NoDiscard]
7+
function test(): int {
8+
return 0;
9+
}
10+
11+
#[\NoDiscard("this is important")]
12+
function test2(): int {
13+
return 0;
14+
}
15+
16+
#[\NoDiscard]
17+
function test3(...$args): int {
18+
return 0;
19+
}
20+
21+
class Clazz {
22+
#[\NoDiscard]
23+
function test(): int {
24+
return 0;
25+
}
26+
27+
#[\NoDiscard("this is important")]
28+
function test2(): int {
29+
return 0;
30+
}
31+
32+
#[\NoDiscard]
33+
static function test3(): int {
34+
return 0;
35+
}
36+
}
37+
38+
$closure = #[\NoDiscard] function(): int {
39+
return 0;
40+
};
41+
42+
$closure2 = #[\NoDiscard] function(): int {
43+
return 0;
44+
};
45+
46+
test();
47+
test2();
48+
test3(1, 2, named: 3);
49+
call_user_func("test");
50+
$fcc = test(...);
51+
$fcc();
52+
53+
$cls = new Clazz();
54+
$cls->test();
55+
$cls->test2();
56+
Clazz::test3();
57+
58+
call_user_func([$cls, "test"]);
59+
60+
$closure();
61+
62+
$closure2();
63+
64+
?>
65+
--EXPECTF--
66+
Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
67+
68+
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
69+
70+
Warning: The return value of function test3() should either be used or intentionally ignored by casting it as (void) in %s on line %d
71+
72+
Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
73+
74+
Warning: The return value of function test() should either be used or intentionally ignored by casting it as (void) in %s on line %d
75+
76+
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
77+
78+
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
79+
80+
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
81+
82+
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
83+
84+
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
85+
86+
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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
#[\NoDiscard]: __call(), __callStatic(), and __invoke().
3+
--FILE--
4+
<?php
5+
6+
class Clazz {
7+
#[\NoDiscard]
8+
public function __call(string $name, array $args): int {
9+
echo "__call({$name})", PHP_EOL;
10+
11+
return strlen($name);
12+
}
13+
14+
#[\NoDiscard]
15+
public static function __callStatic(string $name, array $args): int {
16+
echo "__callStatic({$name})", PHP_EOL;
17+
18+
return strlen($name);
19+
}
20+
21+
#[\NoDiscard]
22+
public function __invoke(string $param): int {
23+
echo "__invoke({$param})", PHP_EOL;
24+
25+
return strlen($param);
26+
}
27+
}
28+
29+
$cls = new Clazz();
30+
$cls->test();
31+
Clazz::test();
32+
$cls('foo');
33+
34+
?>
35+
--EXPECTF--
36+
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
37+
__call(test)
38+
39+
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
40+
__callStatic(test)
41+
42+
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
43+
__invoke(foo)
44+

‎Zend/tests/attributes/nodiscard/003.phpt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
#[\NoDiscard]: Taken from trait.
3+
--FILE--
4+
<?php
5+
6+
trait T {
7+
#[\NoDiscard]
8+
function test(): int {
9+
return 0;
10+
}
11+
}
12+
13+
class Clazz {
14+
use T;
15+
}
16+
17+
$cls = new Clazz();
18+
$cls->test();
19+
20+
?>
21+
--EXPECTF--
22+
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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
#[\NoDiscard]: Native function and method.
3+
--FILE--
4+
<?php
5+
6+
$f = tmpfile();
7+
flock($f, LOCK_SH | LOCK_NB);
8+
fclose($f);
9+
10+
$date = new DateTimeImmutable('now');
11+
$date->setTimestamp(0);
12+
13+
?>
14+
--EXPECTF--
15+
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
16+
17+
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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
#[\NoDiscard]: execute_ex overwritten
3+
--EXTENSIONS--
4+
zend_test
5+
--INI--
6+
zend_test.replace_zend_execute_ex=1
7+
opcache.jit=disable
8+
--FILE--
9+
<?php
10+
11+
#[\NoDiscard]
12+
function test(): int {
13+
return 0;
14+
}
15+
16+
test();
17+
18+
?>
19+
--EXPECTF--
20+
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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
#[\NoDiscard]: execute_internal overwritten
3+
--EXTENSIONS--
4+
zend_test
5+
--INI--
6+
zend_test.observer.execute_internal=1
7+
--FILE--
8+
<?php
9+
10+
$f = tmpfile();
11+
flock($f, LOCK_SH | LOCK_NB);
12+
fclose($f);
13+
14+
?>
15+
--EXPECTF--
16+
<!-- internal enter tmpfile() -->
17+
<!-- internal enter NoDiscard::__construct() -->
18+
19+
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
20+
<!-- internal enter flock() -->
21+
<!-- internal enter fclose() -->

‎Zend/tests/attributes/nodiscard/008.phpt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
#[\NoDiscard]: Combining with #[\Deprecated].
3+
--FILE--
4+
<?php
5+
6+
#[\NoDiscard]
7+
#[\Deprecated]
8+
function test(): int {
9+
return 0;
10+
}
11+
12+
test();
13+
14+
?>
15+
--EXPECTF--
16+
Deprecated: Function test() is deprecated in %s on line %d
17+
18+
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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
#[\NoDiscard]: Combining with #[\Deprecated] (Internal).
3+
--EXTENSIONS--
4+
zend_test
5+
--FILE--
6+
<?php
7+
8+
zend_test_deprecated_nodiscard();
9+
10+
?>
11+
--EXPECTF--
12+
Deprecated: Function zend_test_deprecated_nodiscard() is deprecated, custom message in %s on line %d
13+
14+
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

0 commit comments

Comments
 (0)