Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108950
MIME-Version: 1.0
Date: Tue, 10 Mar 2020 15:33:37 +0100
Message-ID: <CAF+90c_nNFBwwQc+mh6mubT=F6EAtCa28o+f1JRdOk7FnwJ42A@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000045f25e05a081023b"
Subject: Fixing fetch mode for object property access (SimpleXML
 auto-vivification breakage)
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000045f25e05a081023b
Content-Type: text/plain; charset="UTF-8"

Hi internals,

While reviewing the RFC for read-only properties, I encountered the
following long-standing issue once again:

    $foo->bar->baz = 42;

should be the same as

    $bar = $foo->bar;
    $bar->baz = 42;

but isn't, because the "$foo->bar" is compiled as write-fetch rather than
read-fetch. This is incorrect, because here $foo->bar is not modified
itself -- the object it contains is modified.

This does not matter a lot right now, but is problematic when it comes to
readonly properties (and also property accessors). The readonly property
implementation would incorrectly treat the first snippet as an error,
because the PHP engine treats it as a write to $foo->bar.

The historical context for this peculiar behavior is the
"auto-vivification" feature that has been removed in PHP 8:

    $x = new stdClass;
    $x->a->b = 'c';
    var_dump($x);

On PHP 7.4 this results in:

Warning: Creating default object from empty value in /in/MVgRo on line 4
object(stdClass)#1 (1) {
  ["a"]=> object(stdClass)#2 (1) {
    ["b"]=> string(1) "c"
  }
}

That is, it allows a "deep" initialization of the object, even though the
intermediate level has not been initialized.

In PHP 8 this results in an Error exception:

Fatal error: Uncaught Error: Attempt to assign property 'b' of non-object
in /in/MVgRo:4

This change has already happened, and as such no longer poses an issue for
compiling property accesses with correct fetch-mode.

However, a similar behavior still exists in SimpleXML:

$x = simplexml_load_string("<root></root>");
$x->a->b->c = 'Test';
echo $x->asXML();

Results in:

<?xml version="1.0"?>
<root><a><b><c>Test</c></b></a></root>

Here again, an intermediate layer is implicitly initialized.

After fixing the fetch-mode, an Error exception is thrown instead:

Fatal error: Uncaught Error: Attempt to assign property 'c' of non-object
in /home/nikic/php-src/t150.php:3

As such, the behavior will be in line with normal objects in PHP 8. It
should also be noted that the current behavior is undocumented -- the
documentation uses addChild() for this purpose.

After writing out this mail, I'm not sure there is anything to discuss here
really, as this just aligns SimpleXML behavior with an already accepted BC
break in PHP 8, but I guess I should at least let people know about this :)
For reference, a prototype implementation for this change is available at
https://github.com/php/php-src/pull/5250.

Regards,
Nikita

--00000000000045f25e05a081023b--