Allow lazy ghosts to remain partially uninitialized

[iluuu1994]

Fixes GH-19224

Needs a discussion and maybe an RFC. /cc @procodix @arnaud-lb @nicolas-grekas

[arnaud-lb]

This is a nice way to implement lazy properties.

Some thoughts:

I think we have an expectation that after calling zend_lazy_object_init(), restarting the operation will succeed without triggering initialization again. We can now break this expectation by leaving the prop uninitialized and returning true. It would make sense to me if zend_lazy_object_init_ex(obj, prop) errored in case the initializer returned true but didn't initialize prop.

An other issue is that all properties with a default value will be considered non-lazy. We would need a way to specify exactly which props were initialized.

To avoid potential BC breaks I would make this opt-in with a newLazyGhost() flag (so that we don't pass the prop name arg unless the flag is set).

[iluuu1994]

I think we have an expectation that after calling zend_lazy_object_init(), restarting the operation will succeed without triggering initialization again.

I'm not sure if I understood you correctly, but I believe that's how it should work. Notice this condition:

if (Z_TYPE(retval) == IS_TRUE && prop) {

This branch is only hit when we requested a single property. When requesting initialization of the entire object, returning true will not keep the object lazy, and thus won't re-trigger initialization. That said, the code does currently mark the property that was requested to be initialized with zend_lazy_object_init_ex() as lazy if it remains uninitialized. We might want to skip the flagging of these properties with IS_PROP_LAZY, so that repeated access of one property won't keep triggering initialization, potentially causing recursion.

An other issue is that all properties with a default value will be considered non-lazy. We would need a way to specify exactly which props were initialized.

Ah, good point. Edit: Unless we just consider the initialization of the default properties as an implicit part of the first initializer execution. The initializer might depend on these properties, so it would be weird to unset them and then initialize them again, even though technically they should always stay the same.

To avoid potential BC breaks I would make this opt-in with a newLazyGhost() flag (so that we don't pass the prop name arg unless the flag is set).

👍 Would also be a cleaner, more understandable API by avoiding the return true;.

[arnaud-lb]

That said, the code does currently mark the property that was requested to be initialized with zend_lazy_object_init_ex() as lazy if it remains uninitialized. We might want to skip the flagging of these properties with IS_PROP_LAZY, so that repeated access of one property won't keep triggering initialization, potentially causing recursion.

This is the edge case I had in mind :)

More generally we may have code that assumes that the object is fully initialized after a successful call to zend_lazy_object_init().

An other issue is that all properties with a default value will be considered non-lazy. We would need a way to specify exactly which props were initialized.

Ah, good point. Edit: Unless we just consider the initialization of the default properties as an implicit part of the first initializer execution. The initializer might depend on these properties, so it would be weird to unset them and then initialize them again, even though technically they should always stay the same.

I agree that it's useful that properties are set to their default value before calling the initializer, but forcing the initializer to take care of all properties that have a default value during the first call defeats the use-case. In the following example class the initializer would be forced to fetch/initialize both properties when any of them is accessed:

class Node {
    public ?Node $left = null;
    public ?Node $right = null;
}

I think it would be useful if the initializer could specify which properties it has initialized during this call, such that the other ones (that were not already initialized before) are marked as lazy again after the call:

$obj = $reflector->newLazyGhost(function ($obj, $prop) {
    $obj->left; // default value, unless it was initialized before
    $obj->right; // default value, unless it was initialized before
    if ($prop === 'left') {
        $obj->left = fetchLeft();
        return ['left'];
    }
    if ($prop === 'right') {
        $obj->right = fetchRight();
        return ['right'];
    }
    return [];
});

$obj->left; // initializes 'left', leaves 'right' lazy
$obj->right; // initializes 'right', now both props are initialized

Edit: This is wrong, as the initializer has no way to know if other properties were set to their default value, or if they were initialized by the initializer.

An alternative would be to only take care of the accessed property before calling the initializer (initialize it to its default value it any, mark it as non-lazy, leave other properties as lazy). If the initializer accesses any other property, it would call the initializer again for that property. This is more transparent to the initializer, but may have more overhead.