Fix UNEXPECTED() paren mistakes.

[nielsdos]

This corrects the paren placement to the intended one. As these functions use zend_result, the success value is zero. Therefore this has no functional change. The only difference is that this now hints the compiler optimizer correctly.

As this does not fix a bug (can only improve performance), I targeted master.

I found these by grepping for more of cases like #10332.

[TimWolla]

I found these by grepping for more of cases like

Want to use the opportunity to share Coccinelle to PHP once more 😄

The following simple Coccinelle patch should detect misuses of these macros:

@@
expression e;
expression f;
@@

* ((UNEXPECTED(e)) != f)

@@
expression e;
expression f;
@@

* ((UNEXPECTED(e)) == f)

@@
expression e;
expression f;
@@

* ((EXPECTED(e)) != f)

@@
expression e;
expression f;
@@

* ((EXPECTED(e)) == f)

I've ran the patch on ext/ and didn't find anything. For Zend/ I believe you missed one in zend_gc.c. I didn't complete the search, because Zend/zend_vm_execute.h is huge and Coccinelle took ages, before I aborted.

diff -u -p Zend/zend_gc.c /tmp/nothing/zend_gc.c
--- Zend/zend_gc.c
+++ /tmp/nothing/zend_gc.c
@@ -591,7 +591,6 @@ static zend_never_inline void ZEND_FASTC
        if (GC_G(gc_enabled) && !GC_G(gc_active)) {
                GC_ADDREF(ref);
                gc_adjust_threshold(gc_collect_cycles());
-               if (UNEXPECTED(GC_DELREF(ref)) == 0) {
                        rc_dtor_func(ref);
                        return;
                } else if (UNEXPECTED(GC_INFO(ref))) {

Usage: spatch -sp_file unexpected.cocci --include-headers --include-headers-for-types --recursive-includes --dir Zend/

[devnexen]

I would not mind much if it was backported like your previous similar, fix even tough indeed the effect is not really the same. Let's see what other think and once you address the comment.

[Girgias]

I don't really see a harm in backporting this either.

[iluuu1994]

Just my opinion, I don't really mind either way. Generally I prefer not backporting things that don't solve observable issues. For older branches this might trigger a release when there doesn't have to be one. It also increases the diff when looking for potential changes. Either is fine for me for this case.

[mvorisek]

👍 for integrating Coccinelle, it should also detect issues like #10332

[cmb69]

As this does not fix a bug (can only improve performance), I targeted master.

And that is appropriate, in my opinion. #10332 fixed an actual bug, though, due to the way we define UNEXPECTED (double negation of the expression).

[nielsdos]

Thank you all for the reviews.

@TimWolla Oh yeah Coccinelle! I really should learn smpl someday :). I ran your cocci file until it finished and it didn't find any additional cases (well except for the one you already posted).

I guess this PR was closed a bit early since I hadn't had the chance to add the zend_gc.c case, so I'll create a PR for that one shortly :)