On Sun, Feb 19, 2023, at 08:31, Max Kellermann wrote:
> Hi,
>
> I've done a bit of refactoring work around code using "zend_result",
> but I keep wondering why it even exists.
>
> It was added in 1999 by commit 573b46022c46 in a huge 16k line commit
> (as macros, converted to enum in 2012 by commit e3ef84c59bf).
>
> In 1999, C99 was brand new, and the "bool" type had just been
> introduced to the C language - yes, C was 18 years old when a native
> boolean type was added - but PHP didn't switch to C99 for another 19
> years later (b51a99ae358b).
>
> C had a long history of abusing "int" as boolean type, wasting 2 or 4
> bytes for something that could have easily fit in 1 byte. As if that
> wasn't obscure enough, POSIX used 0 for success and -1 for error (and
> missed the chance to return error codes, and instead added a global
> variable for that which caused more headaches). (And don't get me
> started on the horrible strcmp() return value.) Returning errors in C
> is a huge obscure and inconsistent mess; every function does it
> differently.
>
> This is PHP's original definition:
>
> #define SUCCESS 0
> #define FAILURE -1 /* this MUST stay a negative number, or it may effect
> functions! */
>
> This appears to follow the POSIX school of bad error return values.
> There's a comment which makes the thing even more obscure.
>
> Really, why does it need to be negative?
>
> Why does it imitate POSIX instead of using a boolean? (i.e. "int" and
> non-zero for success for old-schoolers)
>
> The obvious way to indicate success or failure (without giving details
> about the nature of the failure) would be to just return "bool". With
> C99, any discussion on "0 or 1" vs "-1 or 0" is obsolete - there is
> now a canonical boolean type that should be used.
>
> Of course, I know already that getting rid of "zend_result" in favor
> of "bool" would be a major API breakage that requires careful
> refactoring of almost all extensions.
>
> I just want to understand why "zend_result" was ever invented and why
> it uses those surprising integer values. The commit message and that
> code comment doesn't explain it.
>
> Rephrased: do you consider it a worthy goal to eventually get rid of
> "zend_result", or do you believe it's good API design that should stay
> forever?
>
> (Yet again I wish PHP was fully C++ - unlike C, C++ has strongly-typed
> enums which cannot be casted implicitly to "int" or "bool"; that makes
> refactoring a lot easier and safer.)
>
> Max
Any type that has only two values is isomorphic to a boolean. However, for us
humans, not all two-valued types are semantically equivalent.
If you have a function like zend_hash_exists(), true and false are directly
meaningful values.
If you have a function like zend_stream_open_function(), SUCCESS and FAILURE
are directly meaningful values.
Now, if you make zend_stream_open_function() return a boolean instead, it's no
longer clear what the return value means. Does a true return value mean that
the stream was opened successfully, or that it failed?
For a PHP programmer, that might sound silly -- of course true means success.
However, in C the common error reporting convention is actually the other way
around: Non-zero return values indicate failure. This means that false
indicates success and true indicates failure. (I'm not kidding you -- I'm
literally working on a project that uses boolean return values with this
convention right now.)
The current guideline for use of bool and zend_result in php-src is that bool
is an appropriate return value for "is" or "has" style functions, which return
a yes/no answer. zend_result is an appropriate return value for functions that
perform some operation that may succeed or fail.
I think that's a pretty reasonable state of things, and don't think there is a
need to change it.
Regards,
Nikita