Hi
Am 2025-02-12 15:07, schrieb Kamil Tekiela:
I'd be honest, I have a very negative attitude towards this proposal
and I'd be voting against it. It seems to me like it's creating a
problem and then trying to find a solution for it.
Given that even Rust with a modern stdlib and ergonomic language features has such an attribute, we do not believe it's an invented problem. With the bulk_process() example, we also tried to provide coherent user-story with a use-case that the readers might've already encountered themselves in the same or a similar fashion. Personally I encountered situations where I wished such an attribute would've warned me both in PHP and JavaScript/TypeScript. Also in C of course, but given that C doesn't have Exceptions, the situation is somewhat different.
For an additional example use-case, consider a function returning a resource object with side-effects which will automatically clean up the resource when being destructed. Specifically a function to spawn a new process or thread:
class Process {
public function __destruct() { /* kill the process */ }
}
#[NoDiscard("the process will be terminated when the Process object goes out of scope")]
function start_process(string $executable): Process {
return new Process($executable);
}
start_process('/my/background/process');
Depending on timing, the process might or might not run to completion before PHP gets around to kill it, for example when a step debugger is attached to PHP, making the bug disappear when debugging.
Related to the above example and also to the flock() function: A developer might want to write a locking function that returns a lock resource that will automatically unlock when it goes out of scope, preventing the unlock from being forgotten. It might not be immediately obvious that the locking function returns a lock resource, especially when it's associated with another object:
$cacheDriver->lock('someCacheEntry');
Is there a $cacheDriver->unlock('someCacheEntry') or will ->lock() return a lock resource? Having the attribute on the lock() method can help.
Searching for language:rust "must_use =" on GitHub reveals that a common use-case in Rust is lazy iterators, where you can call higher-order functions (e.g. map() or filter()), but they will not do anything unless you explicitly “poll” them at least once. As far as I know, Java has lazy streams that behave identically.
A return value is always supposed to be used. If some API is returning
a value that can be safely ignored, it's a badly designed API. If a
We agree that in a greenfield ecosystem, we would make ignoring the return value of a function a warning by default (with an opt-out mechanism), as done in Swift. However in PHP we do not have a greenfield situation, there a number of functions where the return value is useless and only exists for historical reasons. This includes all functions which nowadays have a return-type of just true, which was only added for this singular purpose.
There's cases where ignoring the return value is safe and reasonable, without being a badly designed API. array_pop() would come to mind: If one is only interested in the side-effect of removing the last element in the array, one does not need the return value without necessarily doing something unsafe.
doesn't feel like the solution. In fact, it's creating a problem for
users who want to ignore the value, which you then propose to solve
with (void) cast.
Ignoring the return value of functions having the attribute should be a rare occurrence given the intended use-case of pointing out unsafe situations. But as with static analyzers there needs to solution to suppress warnings, after carefully verifying that the warning is not applicable in a given situation.
the compilation phase. In PHP warnings are runtime errors. The code
should emit an exception instead of a warning. It would also make it
See Volker's reply to Derick.
much easier to handle and you wouldn't need any special construct to
allow users to ignore the new attribute. And I am really not a fan of
Even if an Exception would be thrown, there would be a mechanism to suppress the Exception. A catch-block wouldn't work, because if the Exception is thrown, the function is not executed / control flow is interrupted.
the PHP engine generating E_USER_WARNING which should be reserved only
for warnings triggered by trigger_error.
This follows the lead of the #[\Deprecated] attribute, which emits E_USER_DEPRECATED for userland functions and E_DEPRECATED for native functions, despite being triggered by the engine.
The examples you used don't support the need for the new attribute.
Regarding the DateTimeImmutable methods, you said yourself: "The
methods are badly named, because they do not actually set the updated
value". So your proposal suggests adding a brand new thing to PHP to
deal with bad method names?
DateTimeImmutable is not part of the “Use Cases” section: Our intended use-case is the kind of bulk_process() functionality that is used for all the code snippets. But given that the attribute is also useful for DateTimeImmutable, we made it part of the proposal, without it being part of the intended “user-story”.
This problem should be solved using static analysers, IDE, and proper
code refactoring.
See Volker's reply to Matteo.
Best regards
Tim Düsterhus