Everything

php

Home » Project » Newsgroups » php.internals » Binary string forward compatibility removal »

Binary string forward compatibility removal

php.internals

Pedro Magalhães

9 years ago

Hi internals,

I've created a PR (https://github.com/php/php-src/pull/2187) aiming at
the removal of the binary string forward compatibility.

Reproducing the description of the PR:
----
In version 5.2.1, the b prefix and the (binary) cast were introduced for
forward compatibility with the PHP6 project. As it is known, that
project never came to be. However, these are still accepted by the
language scanner although ignored from then on.

This PR aims at removing those as they are naturally confusing given
that they are simply ignored or, in the case of the cast, the same as
casting to string.

However, I've separated this PR in 2 commits because the removal of the
(binary) cast will bring a more serious BC break issue: On the PHAR
extension, the current default stub for PHAR's makes use of the binary
cast. I've removed it from the default stub and fixed the tests that
made use of it but this means that the old PHARs that make use of this
default stub will be broken.

What seemed to be a simple task actually turned out to be far more
complex given the number of tests that were written with this forward
compatibility in mind.

I would be happy to transform this into an RFC if you think it would be
appropriate.
----

Any feedback would be greatly appreciated.

Best regards,
Pedro Magalhães

François Laupretre

9 years ago

Hi,

it was stated during the pre-7 discussions, that this flag would be kept
for possible future unicode-compliant developments. So, people needing
binary strings are still encouraged to use the flag, even if it is not
used in the current versions.

Regards

François

Le 06/11/2016 à 20:22, Pedro Magalhães a écrit :

Nikita Popov

9 years ago

On Sun, Nov 6, 2016 at 8:56 PM, François Laupretre <francois@php.net> wrote:

> Hi,
>
> it was stated during the pre-7 discussions, that this flag would be kept
> for possible future unicode-compliant developments. So, people needing
> binary strings are still encouraged to use the flag, even if it is not used
> in the current versions.
>
> Regards
>
> François
>

I don't remember any specific on-list discussion on this topic, but my
general understanding is that at this point the unicode by default approach
has sailed. We may see something like u"" literals in the future (iirc this
was suggested during the UString discussion), but we quite certainly are
not going to change the default interpretation of strings -- it would cause
too much breakage.

Nikita

Andrew Faulds

9 years ago

Hi Pedro,

I don't support this removal.

Currently, yes, the flag doesn't do anything, and it's unlikely to do so
in future, as Nikita points out.

However, it's not improbable that we might see a `u` flag in future, and
if that was added, `b` might still be useful as an explicit counterpart.

Anyway, this feels like deprecation for its own sake. As you point out,
removing this breaks widely-deployed code. When the benefit of removal
is negligible, and it breaks existing code, I don't think it can be
justified.

Thanks.

--
Andrea Faulds
https://ajf.me/

Joe Watkins

9 years ago

Morning Andrea,

Who is widely deploying something that does absolutely nothing ?

The only reason to keep it would be that we are going to change the
default representation, as pointed out we're not.

This cannot have been left in for considered reasons, it was just
forgotten about.

Cheers
Joe

On Mon, Nov 7, 2016 at 1:22 AM, Andrea Faulds <ajf@ajf.me> wrote:

Andrew Faulds

9 years ago

Hi,

Joe Watkins wrote:

> Morning Andrea,
>
>      Who is widely deploying something that does absolutely nothing ?

PHAR uses (binary), or so Pedro's email says.

>
>      The only reason to keep it would be that we are going to change the
> default representation, as pointed out we're not.

If we were adding it tomorrow, maybe, but `b` and the (binary) cast are
already here. A high standard must be met for removing features. These
aren't a security problem, nor a maintenance burden. There's not a good
case for removing them and breaking backwards-compatibility

>
>      This cannot have been left in for considered reasons, it was just
> forgotten about.

If it was to be removed, the time to do so would have been right after
PHP 6's failure. But it's been years now. I'm unconvinced there's a
strong case to remove it.

Thanks for your reply.

--
Andrea Faulds
https://ajf.me/

Joe Watkins

9 years ago

Afternoon Andrea,

A case can be made that the binary cast might at some point do
something: "(binary) $ustring"

There is no case whatever for the literal b prefix, it will *never*
have a function if we are not changing the default string representation.

If we do adopt a literal prefix for unicode strings, it's going to be
rather confusing to learn that u"" does something and b"" is also
supported, but b"" doesn't, and has never done anything.

OP: I think this should be split into two RFC's and voted on
separately, it certainly can't be merged by anyone as a PR.

Cheers
Joe



On Mon, Nov 7, 2016 at 11:26 AM, Andrea Faulds <ajf@ajf.me> wrote:

Andrew Faulds

9 years ago

Hi,

Joe Watkins wrote:

> Afternoon Andrea,
>
>      A case can be made that the binary cast might at some point do
> something: "(binary) $ustring"
>
>      There is no case whatever for the literal b prefix, it will *never*
> have a function if we are not changing the default string representation.
>
>      If we do adopt a literal prefix for unicode strings, it's going to be
> rather confusing to learn that u"" does something and b"" is also
> supported, but b"" doesn't, and has never done anything.

Well, that's not quite fair. b"" and "" are equivalent, u"" wouldn't be.

It's only as useless as, say, the unary + operator. I can imagine it
being used to be explicit about using a binary string.

In any case, I don't think that it not doing anything that a bare string
literal does makes enough of a case to remove it. It's going to bite
some people if we remove this, and for what gain?

>
>      OP: I think this should be split into two RFC's and voted on
> separately, it certainly can't be merged by anyone as a PR.

The two are related, if we're to get rid of one, we should get rid of
both. If there's BC concerns for (binary), I suspect there's also
breakage for b"". (binary) is also “useless”, insofar as you could just
use (string).

Thanks.

--
Andrea Faulds
https://ajf.me/

Niklas Keller

9 years ago

2016-11-06 20:22 GMT+01:00 Pedro Magalhães <mail@pmmaga.net>:

> Hi internals,
>
> I've created a PR (https://github.com/php/php-src/pull/2187) aiming at
> the removal of the binary string forward compatibility.
>
> Reproducing the description of the PR:
> ----
> In version 5.2.1, the b prefix and the (binary) cast were introduced for
> forward compatibility with the PHP6 project. As it is known, that project
> never came to be. However, these are still accepted by the language scanner
> although ignored from then on.
>
> This PR aims at removing those as they are naturally confusing given that
> they are simply ignored or, in the case of the cast, the same as casting to
> string.
>
> However, I've separated this PR in 2 commits because the removal of the
> (binary) cast will bring a more serious BC break issue: On the PHAR
> extension, the current default stub for PHAR's makes use of the binary
> cast. I've removed it from the default stub and fixed the tests that made
> use of it but this means that the old PHARs that make use of this default
> stub will be broken.
>
> What seemed to be a simple task actually turned out to be far more complex
> given the number of tests that were written with this forward compatibility
> in mind.
>
> I would be happy to transform this into an RFC if you think it would be
> appropriate.
> ----
>
> Any feedback would be greatly appreciated.
>
> Best regards,
> Pedro Magalhães
>
>
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

No matter whether we choose to merge this somewhere in the future or not,
there's no reason to have (binary) in the phar stub anymore.

I have just opened a PR to remove the (binary) casts there again, as we no
longer need that forward compatibility:
https://github.com/php/php-src/pull/2190

As for the removal itself: There's no particular reason to remove it now,
but we could at least issue a deprecation warning.

Regards, Niklas

Pedro Magalhães

9 years ago

On 2016-11-06 20:22, Pedro Magalhães wrote:

> Hi internals,
>
> I've created a PR (https://github.com/php/php-src/pull/2187) aiming at
> the removal of the binary string forward compatibility.
>
> Reproducing the description of the PR:
> ----
> In version 5.2.1, the b prefix and the (binary) cast were introduced
> for forward compatibility with the PHP6 project. As it is known, that
> project never came to be. However, these are still accepted by the
> language scanner although ignored from then on.
>
> This PR aims at removing those as they are naturally confusing given
> that they are simply ignored or, in the case of the cast, the same as
> casting to string.
>
> However, I've separated this PR in 2 commits because the removal of
> the (binary) cast will bring a more serious BC break issue: On the
> PHAR extension, the current default stub for PHAR's makes use of the
> binary cast. I've removed it from the default stub and fixed the tests
> that made use of it but this means that the old PHARs that make use of
> this default stub will be broken.
>
> What seemed to be a simple task actually turned out to be far more
> complex given the number of tests that were written with this forward
> compatibility in mind.
>
> I would be happy to transform this into an RFC if you think it would
> be appropriate.
> ----
>
> Any feedback would be greatly appreciated.
>
> Best regards,
> Pedro Magalhães

Hello, internals!

Based on the feedback here and on GitHub, this change should go through
a deprecation phase and there should be an RFC about it.

This deprecation is also covered on Nikita's deprecations RFC but we
agreed to take this on separately as it has the BC break issue for old
PHAR's.

Given that, I would like to request for rfc karma on the wiki for my
user: "pmmaga"

Thank you in advance,
Pedro Magalhães