Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100256
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.223.175 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <11BCF7F8-4AEE-43FE-9D26-7D64C54EA382@gmail.com>
References: <C8CF5498-7451-4BDC-935F-17D010DBA6B6@bk.ru> <D74A9128-95B5-4F35-BD89-C474C2FF923A@bk.ru>
 <C430C088-8A48-4124-9814-9F39A3B2E5C6@gmail.com> <11BCF7F8-4AEE-43FE-9D26-7D64C54EA382@gmail.com>
Date: Sat, 19 Aug 2017 15:30:19 +0200
Message-ID: <CAF+90c8G--Fi1zQ_REw4UwYEAnSBc8oEyeGxq-eQVAYR_yDT8w@mail.gmail.com>
To: Andrew Nester <andrew.nester.dev@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a113eeb183cdf1e05571b3f96"
Subject: Re: [PHP-DEV] Re: [Request][Discussion] Double value as array key improvement
From: nikita.ppv@gmail.com (Nikita Popov)

--001a113eeb183cdf1e05571b3f96
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Aug 17, 2017 at 5:03 PM, Andrew Nester <andrew.nester.dev@gmail.com=
>
wrote:

>
>
> > 13 =D0=B0=D0=B2=D0=B3. 2017 =D0=B3., =D0=B2 21:39, Andrew Nester <andre=
w.nester.dev@gmail.com>
> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):
> >
> >
> >
> >> 11 =D0=B0=D0=B2=D0=B3. 2017 =D0=B3., =D0=B2 15:53, Andrew Nester <newa=
ltgroup@bk.ru> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):
> >>
> >>
> >>> On Aug 11, 2017, at 2:10 PM, Andrew Nester <newaltgroup@bk.ru> wrote:
> >>>
> >>> Hello everyone!
> >>>
> >>> I was working on following request https://bugs.php.net/bug.php?
> id=3D75053 which resulted in following pull request
> https://github.com/php/php-src/pull/2676
> >>>
> >>> The problem here is following: when we=E2=80=99re using large numbers=
 as array
> index when adding new elements it could overwrite already existing value.
> >>> Assume we have 2 indexes 5076964154930102272 and
> 999999999999999999999999999999 with different value set for them.
> >>>
> >>> Because 999999999999999999999999999999 is larger than maximum long in=
t
> number for 64-bit systems, it will be converted to double. (corresponding
> code here https://github.com/php/php-src/blob/master/Zend/zend_
> language_scanner.l#L1648)
> >>> But when double value is used as array indexes, it is converted to
> long integer. (f.e., code is here https://github.com/php/php-
> src/blob/master/Zend/zend_execute.c#L1573)
> >>> At this case it causes overflow and we=E2=80=99ve got index equal to
> 5076964154930102272 and as a result - we=E2=80=99re overwriting previousl=
y set
> value.
> >>>
> >>> My suggestion is following:
> >>> 1) when double key is less than maximum possible long integer -
> convert it to integer
> >>> 2) if it=E2=80=99s larger - convert it to string.
> >>>
> >>> That=E2=80=99s what implemented in proposed PR.
> >>>
> >>> Another possible option is just to throw warning in this case
> (proposed by Nikita Popov)
> >>>
> >>> I would happy to hear any feedback and suggestions about this solutio=
n.
> >>> Thanks!
> >>
> >> Here is the alternative solution which emits E_WARNING in case of
> integer array index overflow.
> >> https://github.com/php/php-src/pull/2677
> >
> > My preferred solution is 2nd one (emitting warning) as it more obvious
> for users, doesn't break previous behaviour.
> >
> > Cheers,
> > Andrew
>
> Hello internals!
>
> I was working on solution for the problem of double to int conversion for
> array indices and would like to create an RFC for proposed solution -
> emitting warning when integer overflow happens during double to int
> conversion.
>
> Does it look like good idea?
>
> Thanks!


Sounds good to me. Something you might want to consider is to also throw a
warning if the floating point number is not an exact integer. For example
allow a silent cast of 42.0 to 42, but throw a warning if 42.5 is used as
an index (or worse, 42.999999999, in which case it likely isn't doing what
the programmer thinks it's doing).

Nikita

--001a113eeb183cdf1e05571b3f96--