Fix GH-24063: SCCP merges 0.0 and -0.0 into a single constant

[lazerg]

SCCP joins phi values with zend_is_identical(), which compares doubles with ==, so 0.0 and -0.0 count as the same value. A ternary like $n < 0 ? -0.0 : 0.0 then gets folded to whichever constant the first branch held, and the condition disappears. The same thing happens for constant arrays and for elements of partial arrays.

The join now compares doubles bitwise (like compact_literals.c already does), recursing into arrays, and falls back to zend_is_identical() for everything else.

Fixes GH-24063

[LamentXU123]

What is the usefulness of this one line helper while you can just use !sccp_is_identical in it's only call site?

[lazerg]

It's the zend_hash_compare() callback, which has to return 0 on a match, so !sccp_is_identical can't be passed directly.

[LamentXU123]

What I mean is I doubt this design. what do you think of

static int sccp_values_differ(const void *p1, const void *p2)
{
    const zval *a = p1;
    const zval *b = p2;

    if (Z_TYPE_P(a) == IS_DOUBLE && Z_TYPE_P(b) == IS_DOUBLE) {
        return memcmp(&Z_DVAL_P(a), &Z_DVAL_P(b), sizeof(double)) != 0;
    }

    if (Z_TYPE_P(a) == IS_ARRAY && Z_TYPE_P(b) == IS_ARRAY) {
        return Z_ARRVAL_P(a) != Z_ARRVAL_P(b)
            && zend_hash_compare(
                Z_ARRVAL_P(a), Z_ARRVAL_P(b),
                sccp_values_differ, /* ordered */ true) != 0;
    }

    return !zend_is_identical(a, b);
}

ps: I remove my next review because I don't prefer the original design of this.

[lazerg]

Done in 90d0706.

[LamentXU123]

Thank you. Okay now I want to hear from others about this. /cc @iluuu1994 @arnaud-lb

[arnaud-lb]

Looks good to me!

[arnaud-lb]

In case we add more persistent containers in the future, an assertion like this would be helpful:

    ZEND_ASSERT((1 << Z_TYPE_P(a)) & MAY_BE_UNDEF|MAY_BE_NULL|MAY_BE_BOOL|MAY_BE_LONG|MAY_BE_STRING);

(We can check that Z_TYPE_P(a) == Z_TYPE_P(b) at the beginning of the function to simplify the rest.)

[lazerg]

Added in 478d95a. The assert also lets PARTIAL_ARRAY/PARTIAL_OBJECT through, because nested partial arrays reach it via join_hash_tables().

[arnaud-lb]

Thank you!