Use correct fetch mode for object modification

[nikic]

// Compile
$a->b->c = 'd';
// The same as
$b = $a->b;
$b->c = 'd';

Previously, $a->b was treated as a write fetch.

This is an updated version of #3865. The SimpleXML issue still exists.

[kocsismate]

Wow, it was really-really quick! :)

[nikic]

I think we need to bite the bullet here and accept the SimpleXML breakage. The fact that you can do this kind of deep automatic intialization is undocumented (the documentation uses addChild and friends), and we have removed similar auto-vivification behavior for normal objects in PHP 8.

[nikic]

For comparison:

<?php

$x = new stdClass;
$x->a->b->c = 'd';
var_dump($x);

gives on 7.4:

Warning: Creating default object from empty value in /in/AiAoY on line 4

Warning: Creating default object from empty value in /in/AiAoY on line 4
object(stdClass)#1 (1) {
  ["a"]=>
  object(stdClass)#2 (1) {
    ["b"]=>
    object(stdClass)#3 (1) {
      ["c"]=>
      string(1) "d"
    }
  }
}

and on PHP 8.0:

Fatal error: Uncaught Error: Attempt to modify property 'b' of non-object in /in/AiAoY:4
Stack trace:
#0 {main}
  thrown in /in/AiAoY on line 4

It seems like the SimpleXML issue is completely analogous to this change. The only difference is that SimpleXML currently does not throw the warning, but we can't do anything about that at this time.

[dstogov]

This may miss support for IS_INDIRECT.

[dstogov]

I don't have strong opinion about this PR. Technically the patch is fine. On the other hand, it changes behavior, and I don't completely understand the reason. Should this help implementing read-only properties?

[dstogov]

It's going to be hard to explain why $a[2]->x = 2; warns about undefined variable but $a[2][2] = 2; does not.

[nikic]

It may be hard to explain why arrays and objects have different semantics in the first place, but given that they do, this seems more consistent:

$a[0] = 42 where $a is undefined is considered legal and does not throw any notices or errors. $a->x = 42 is considered illegal and throws a warning in PHP 7 and exception in PHP 8. In this case, the undefined variable notice indicates the "root cause" that needs to be fixed, while the "Attempt to assign property 'b' of non-object" is a followup error caused by it.

[nikic]

Should this help implementing read-only properties?

That's right. The basic problem is that

$x = $object->readonlyProperty;
$x->property = 42;
// and
$object->readonlyProperty->property = 42;

should both be allowed. The first case is automatically handled correctly, because the property is fetched for R. The second case is handled incorrectly, because the property is fetched for W.

Workarounds for this are possible (allowing W/RW fetches in the first place, and then adding extra flags to the property assignment if it is "real"), but I think it is much cleaner to emit the correct fetch mode in the first place.

We also have some existing issues in this area, but they are much less visible, because we have some "best effort" workarounds. For example, if you write something like this:

<?php

class Test {
    public function __get($name) {
        return 42;
    }
}

$test = new Test;
$foo =& $test->foo;

You get:

Notice: Indirect modification of overloaded property Test::$foo has no effect

If you write:

<?php

class Test {
    public function __get($name) {
        return new stdClass;
    }
}

$test = new Test;
$foo =& $test->foo;

You don't get a notice. The reason is that we have special-cased objects here to prevent a spurious warning if you write $test->foo->bar = 42. It's not part of this patch, but that can be removed afterwards.

[dstogov]

OK. I don't object against this.