Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119753
MIME-Version: 1.0
References: <CAFPFaMJ1opg7=SZ2ovFiAqjO_z1avpcNPFWTOoOqjhptoxSvcg@mail.gmail.com>
 <9318094F-139C-4392-8307-1C42B327CDD6@cschneid.com>
In-Reply-To: <9318094F-139C-4392-8307-1C42B327CDD6@cschneid.com>
Date: Tue, 28 Mar 2023 13:42:30 +0100
Message-ID: <CAFPFaMLnmkfM7Y5t-E-Sz0y1ZLxDCeubGd79ngqa0e26JdZeAA@mail.gmail.com>
To: Christian Schneider <cschneid@cschneid.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000ff85fa05f7f531a5"
Subject: Re: [PHP-DEV] [RFC] Define proper semantics for range() function
From: george.banyard@gmail.com ("G. P. B.")

--000000000000ff85fa05f7f531a5
Content-Type: text/plain; charset="UTF-8"

On Tue, 28 Mar 2023 at 08:19, Christian Schneider <cschneid@cschneid.com>
wrote:

> Am 28.03.2023 um 00:36 schrieb G. P. B. <george.banyard@gmail.com>:
> > I therefore propose the "Define proper semantics for range() function"
> RFC
> > to address the unintuitive behaviour that sees no usage and/or hide bugs:
> > https://wiki.php.net/rfc/proper-range-semantics
> >
> > The change propose to throw TypeErrors and ValueErrors for case where I
> > couldn't find occurrences in the wild and hide bugs, and emit some
> > E_WARNINGs for cases that are hard to detect via static analysis.
>
> I think it makes sense to clean up the range() function, thanks!
>
> There are two cases I would handle differently:
> - I'm not sure why a negative step for $start > $end is considered wrong,
> I consider range(10, 0, -2) at least as logical/readable as using a
> positive decrement of 2. Not requiring a sign for steps seems weirder to me
> but that's something we cannot change. *BUT* if it is the result of a
> calculation it seems wrong to require an abs() around it. I do see the
> reason for a warning/error when $start < $end and $step < 0.
>

Considering the only other programming language that I know of that has a
range() function that accepts a step argument is Python, and its behaviour
is IMHO worse.
For increasing ranges it requires a positive step, and if not just
generates an empty range.
For decreasing ranges it requires a negative step, and if not just
generates an empty range (this applies even if using the default step value
of 1 which is bonkers).

Making it a requirement to pass a negative step is definitely out of the
question.
Making it okay to use negative steps *only* for decreasing ranges could be
sensible, but we check for the step parameter way before we look into the
boundary values because those are different for int, float and string
boundaries.
Moreover, I personally find it weirder to require a sign for negative steps
as for me a step is something that *must* be positive, and at least Kotlin
seems to somewhat agree with me looking around at
https://kotlinlang.org/docs/ranges.html#progression and playing with the
source code
Namely:
for (i in 4 downTo 1 step 2) print(i)

> 42

for (i in 4 downTo 1 step -2) print(i)

> Exception in thread "main" java.lang.IllegalArgumentException: Step must
be positive, was: -2.


> - Values of '' or null in integer context (e.g. range(null, 10, 2)) should
> IMHO emit a warning first, not directly be changed to a TypeError. The
> usual BC / migration concern :-)
>

When null is used, no TypeError is emitted, just the "usual" null to scalar
deprecation that happens since
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg got accepted.
But I'll add an example to the RFC and a test case in the PR.

Trying to figure out if an empty string was used with another string
boundary is tedious, as this information needs to somehow get carried
around.
A previous iteration of the PR used to convert empty strings to 0 with a
warning, but considering the analysis I decide to just make this a
ValueError as it doesn't seem that empty strings are actually used in
practice.
But this is an easy revert, and I'm not really bound to this decision.

Best regards,

George P. Banyard

--000000000000ff85fa05f7f531a5--