Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:107918
MIME-Version: 1.0
References: <CAFPFaMLfau0zRMy5eJtoF1orTxyOanT+czNNyKptZBA+=LdNRw@mail.gmail.com>
In-Reply-To: <CAFPFaMLfau0zRMy5eJtoF1orTxyOanT+czNNyKptZBA+=LdNRw@mail.gmail.com>
Date: Tue, 17 Dec 2019 08:06:32 +0100
Message-ID: <CABdc3Wq74HGrOFAaMN1eBE6mJacwd_XBjVaoZwtqPkv0ykMMTw@mail.gmail.com>
To: George Peter Banyard <girgias@php.net>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000080e9440599e0f8af"
Subject: Re: [PHP-DEV] Allowing variable strings to be defined for a string offset
From: michal.brzuchalski@gmail.com (=?UTF-8?Q?Micha=C5=82_Brzuchalski?=)

--00000000000080e9440599e0f8af
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

pon., 16 gru 2019 o 10:58 George Peter Banyard <girgias@php.net> napisa=C5=
=82(a):

> Greetings internals,
>
> I'm here to get internals's opinion about allowing a string offset to be
> replaced with arbitrary strings.
>
> Since forever strings with more then one byte have been truncated silentl=
y
> to one byte. The case with empty string *kinda* was existent but had a
> buggy behaviour (see bug 71572 [1]) and as such has been turned into a
> warning in PHP 7.1.0, and is meant to be promoted to an Error Exception p=
er
> the Reclassifying engine warnings RFC. [2]
> An illustration of both these cases is available on 3v4l.org [3]
> <https://3v4l.org/O0nEM>
>
> I've got an implementation ready as a pull request on GitHub [4] (which
> still needs some polishing for it to make CI happy and fix the various
> leaks).
>
> However, the question is if this behaviour is desirable. Moreover, the
> silent truncation of strings with more than one byte should be changed to
> the same severity as the empty string case; i.e. an Error Exception. This
> seems reasonable due to the possible loss of data.
>
> What ever the decision is, a BC break is to be expected. As code which
> inadvertently tries to assign multiple bytes to an offset will know have
> all those bytes in the string whereas before only the first one was used =
to
> replace the byte at the designated offset. On the other hand the
> introduction of an Error Exception is obviously a BC break but as it poin=
ts
> out to some kind of logic error.
> I would assume the impact to be minimal for both of these case.
>
> Any opinions?
>
> Best regards
>
> George Peter Banyard
>
> [1] https://bugs.php.net/bug.php?id=3D71572
> [2] https://wiki.php.net/rfc/engine_warnings
> [3] https://3v4l.org/O0nEM
> [4] https://github.com/php/php-src/pull/5013


Allowing to unset string offset with empty string assignment in your patch
will not trigger a Warning anymore,
but using unset with string offset currently emits "Fatal error: Uncaught
Error: Cannot unset string offset".
An expected result is to be the same but an effect radically different as
you can see [1].

IMHO allowing string offset to be unset using unset construct could be
allowed to ensure consistency then.

Any thoughts on this?

BR,
--
Micha=C5=82 Brzuchalski

[1] https://3v4l.org/0Ol3N

--00000000000080e9440599e0f8af--