Handle indirect zvals in SplFixedArray::__serialize

[nielsdos]

Follow-up for GH-10907 and GH-10921.
I didn't notice it at first but there's another issue that was introduced in 8.2: INDIRECT zvals are not handled properly and so the serialization doesn't contain their value. I didn't notice that when writing my test, sorry about that.

[Girgias]

I'm still very much confused as to what indirect zvals are, but LGTM! :)

[nielsdos]

Thanks for reviewing.

I'm still very much confused as to what indirect zvals are, but LGTM! :)

Basically the same as a reference, but without a refcount and for internal use only. Used here for the dynamic properties table.

[Girgias]

Thanks for reviewing.

I'm still very much confused as to what indirect zvals are, but LGTM! :)

Basically the same as a reference, but without a refcount and for internal use only. Used here for the dynamic properties table.

Ahhh okay so "internal reference" is how I'll keep this in mind then, but I wonder if we are not missing indirect zvals in a lot of potential other places :/

[nielsdos]

I found an additional problem that already existed, but got worse when I tried fixing the original problem of this PR. The wrong hashtable was used. It can still be stale. We should really use the object properties hash table (see my latest commit).
So I reverted what I had merged and I would like to request your re-review. Sorry for the mess-up.

[kocsismate]

Can you please describe me when the key is null? I wasn't be able to understand it 100% based on the comment above

[nielsdos]

I see I removed the comment, I need to place that back.

In any case: if var_dump (or something else that rebuilds the properties table) has been called before serialize(), then the properties table will include entries for the array elements. In those cases the key == NULL, and h == the index of the element.

For example:

<?php
$test = new SplFixedArray(2);
var_dump($test);
var_dump(serialize($test)); // If you add a printf to the foreach loop you'll see that the entries for array index 0 and 1 are also printed. But this does not happen if you remove the preceding var_dump, because then the properties table hasn't been rebuilt yet.

[nielsdos]

Also the difference between this version of the patch and the original is the following:
In the original patch I only added support for the INDIRECT zvals by appending _IND.
Shortly after merging I noticed a problem, so I reverted the merge and re-opened this PR.
The problem I noticed is that the code was using the wrong HashTable, their entries could still be stale (see the most recent commit which adds a testcase for this).

[kocsismate]

Thanks for the explanation! The new code comment is very helpful!

[Girgias]

So we weren't actually using the object properties but just what SPL kept track off?

Still looks sensible and good to me. But clarifying the NULL key case might be good. :)

[nielsdos]

So we weren't actually using the object properties but just what SPL kept track off?

Indeed.

[Girgias]

Okay, I think we are finally good to merge this!

[nielsdos]

Thanks. I think I'll wait a bit until @kocsismate also reviews, I don't want to do another merge-revert dance 😅

[kocsismate]

LGTM

[nielsdos]

Thanks for the review to you both :) I'm going to change the commit title on merge though, because it does more than fixing indirect zvals as it also fixes the stale property problem.
Something like "Handle indirect zvals and use up-to-date property table SplFixedArray::__serialize"