[RFC] [Vote] Base convert changes

5 years ago by Scott Dutton — view source

unread

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

5 years ago by Joe Watkins — view source

unread

The implementation of this does not look ready, there are conflicts so I
can't test it locally, but last time CI ran there were many failures.

Cheers
Joe

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

5 years ago by Scott Dutton — view source

unread

Hi Joe,

I will take a look at conflicts. The failures are extreme value checks
which are a result of allowing the negative numbers. If the negative
numbers one passes I will fix all tests and add some more for the
negative values. The tests fail because of the unsigned -> signed change
(but as you say there were quite a lot of tests).

Would it be easier for 2 prs ? one for each vote ?

Thanks

Scott

The implementation of this does not look ready, there are conflicts
so I can't test it locally, but last time CI ran there were many
failures.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:24, Scott Dutton scott@exussum.co.uk
wrote:

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements [1]

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--

[2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements
[2] http://www.php.net/unsub.php

5 years ago by Joe Watkins — view source

unread

There should probably be a PR targeting 7.4 with the implementation of
"Error on ignored characters" as proposed for 7.4, and a PR targeting
master implementing "Error on ignored characters" with exception change and
implementing "Allow negative arguments".

None of these PR's should cause tests to fail, and where new untested
behaviour is introduced the PR should include tests for that.

Cheers
Joe

Hi Joe,

I will take a look at conflicts. The failures are extreme value checks
which are a result of allowing the negative numbers. If the negative
numbers one passes I will fix all tests and add some more for the
negative values. The tests fail because of the unsigned -> signed change
(but as you say there were quite a lot of tests).

Would it be easier for 2 prs ? one for each vote ?

Thanks

Scott

The implementation of this does not look ready, there are conflicts
so I can't test it locally, but last time CI ran there were many
failures.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:24, Scott Dutton scott@exussum.co.uk
wrote:

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements [1]

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--

[2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements
[2] http://www.php.net/unsub.php

5 years ago by Joe Watkins — view source

unread

It will likely be easier if the Allow negative numbers implementation
(targetting master only) is entirely separate from the Error on ignored
characters implementation,.

Cheers
Joe

There should probably be a PR targeting 7.4 with the implementation of
"Error on ignored characters" as proposed for 7.4, and a PR targeting
master implementing "Error on ignored characters" with exception change and
implementing "Allow negative arguments".

None of these PR's should cause tests to fail, and where new untested
behaviour is introduced the PR should include tests for that.

Cheers
Joe

Hi Joe,

I will take a look at conflicts. The failures are extreme value checks
which are a result of allowing the negative numbers. If the negative
numbers one passes I will fix all tests and add some more for the
negative values. The tests fail because of the unsigned -> signed change
(but as you say there were quite a lot of tests).

Would it be easier for 2 prs ? one for each vote ?

Thanks

Scott

The implementation of this does not look ready, there are conflicts
so I can't test it locally, but last time CI ran there were many
failures.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:24, Scott Dutton scott@exussum.co.uk
wrote:

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements [1]

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--

[2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements
[2] http://www.php.net/unsub.php

5 years ago by Scott Dutton — view source

unread

Hi Joe,

I will have a look at splitting the PR, I am not at a computer where i
can code today though so will be tomorrow at the earliest.

The Negative numbers will be a fair amount of work to make the tests
pass, would this still need to be done if the RFC doesnt pass ? I am
happy to do this work if it looks like it will pass, the reasons it
fails are outlined in the RFC as BC breaks. The tests seem to test the
values which make it fail more than I have seen other code use these
values.

887-939 are ignoring invalid input changes, everything else is negative
numbers

Hope that helps

Scott

There should probably be a PR targeting 7.4 with the implementation
of "Error on ignored characters" as proposed for 7.4, and a PR
targeting master implementing "Error on ignored characters" with
exception change and implementing "Allow negative arguments".

None of these PR's should cause tests to fail, and where new untested
behaviour is introduced the PR should include tests for that.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:43, Scott Dutton scott@exussum.co.uk
wrote:

Hi Joe,

I will take a look at conflicts. The failures are extreme value
checks
which are a result of allowing the negative numbers. If the negative
numbers one passes I will fix all tests and add some more for the
negative values. The tests fail because of the unsigned -> signed
change
(but as you say there were quite a lot of tests).

Would it be easier for 2 prs ? one for each vote ?

Thanks

Scott

The implementation of this does not look ready, there are
conflicts
so I can't test it locally, but last time CI ran there were many
failures.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:24, Scott Dutton scott@exussum.co.uk
wrote:

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements [1] [1]

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it
doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF',
16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--

[2] [2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements [1]
[2] http://www.php.net/unsub.php [2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements
[2] http://www.php.net/unsub.php

5 years ago by Nikita Popov — view source

unread

Hi Joe,

I will have a look at splitting the PR, I am not at a computer where i
can code today though so will be tomorrow at the earliest.

The Negative numbers will be a fair amount of work to make the tests
pass, would this still need to be done if the RFC doesnt pass ? I am
happy to do this work if it looks like it will pass, the reasons it
fails are outlined in the RFC as BC breaks. The tests seem to test the
values which make it fail more than I have seen other code use these
values.

887-939 are ignoring invalid input changes, everything else is negative
numbers

scripts/dev/bless_tests.php can be used to automatically update expected
test output. Doesn't work for all tests (those with many manual wildcards
for example), but may save you some work.

Nikita

Hope that helps

Scott

There should probably be a PR targeting 7.4 with the implementation
of "Error on ignored characters" as proposed for 7.4, and a PR
targeting master implementing "Error on ignored characters" with
exception change and implementing "Allow negative arguments".

None of these PR's should cause tests to fail, and where new untested
behaviour is introduced the PR should include tests for that.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:43, Scott Dutton scott@exussum.co.uk
wrote:

Hi Joe,

I will take a look at conflicts. The failures are extreme value
checks
which are a result of allowing the negative numbers. If the negative
numbers one passes I will fix all tests and add some more for the
negative values. The tests fail because of the unsigned -> signed
change
(but as you say there were quite a lot of tests).

Would it be easier for 2 prs ? one for each vote ?

Thanks

Scott

The implementation of this does not look ready, there are
conflicts
so I can't test it locally, but last time CI ran there were many
failures.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:24, Scott Dutton scott@exussum.co.uk
wrote:

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements [1] [1]

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP 8) when base_convert encounters something it
doesnt
know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF',
16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--

[2] [2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements [1]
[2] http://www.php.net/unsub.php [2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements
[2] http://www.php.net/unsub.php

5 years ago by Scott Dutton — view source

unread

Ah thats brilliant, I will take a look at updating the tests when I
split the PR tomorrow

Thanks Nikita

Scott

On Wed, Jun 19, 2019 at 10:06 AM Scott Dutton scott@exussum.co.uk
wrote:

Hi Joe,

I will have a look at splitting the PR, I am not at a computer where
i
can code today though so will be tomorrow at the earliest.

The Negative numbers will be a fair amount of work to make the tests
pass, would this still need to be done if the RFC doesnt pass ? I am
happy to do this work if it looks like it will pass, the reasons it
fails are outlined in the RFC as BC breaks. The tests seem to test
the
values which make it fail more than I have seen other code use these
values.

887-939 are ignoring invalid input changes, everything else is
negative
numbers

scripts/dev/bless_tests.php can be used to automatically update
expected test output. Doesn't work for all tests (those with many
manual wildcards for example), but may save you some work.

Nikita

Hope that helps

Scott

There should probably be a PR targeting 7.4 with the
implementation
of "Error on ignored characters" as proposed for 7.4, and a PR
targeting master implementing "Error on ignored characters" with
exception change and implementing "Allow negative arguments".

None of these PR's should cause tests to fail, and where new
untested
behaviour is introduced the PR should include tests for that.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:43, Scott Dutton scott@exussum.co.uk
wrote:

Hi Joe,

I will take a look at conflicts. The failures are extreme value
checks
which are a result of allowing the negative numbers. If the
negative
numbers one passes I will fix all tests and add some more for the
negative values. The tests fail because of the unsigned -> signed
change
(but as you say there were quite a lot of tests).

Would it be easier for 2 prs ? one for each vote ?

Thanks

Scott

The implementation of this does not look ready, there are
conflicts
so I can't test it locally, but last time CI ran there were
many
failures.

Cheers
Joe

On Wed, 19 Jun 2019 at 09:24, Scott Dutton
scott@exussum.co.uk
wrote:

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements [1] [1] [1]

Two votes, one to raise a deprecated error in PHP7.4 (raised
to
exception in PHP 8) when base_convert encounters something it
doesnt
know how to convert.

Second vote is to allow negative numbers, eg
base_convert('-FF',
16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--

[2] [2]
[2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements [1] [1]
[2] http://www.php.net/unsub.php [2] [2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements [1]
[2] http://www.php.net/unsub.php [2]

--

[2]

Links:

[1] https://wiki.php.net/rfc/base_convert_improvements
[2] http://www.php.net/unsub.php

5 years ago by me@jhdxr.com — view source

unread

Hi Scott,

Could you clarify what will happen if multiple negative sign occurs. I didn't find it in the RFC.
e.g. base_convert('--ff', 16, 10)

Thanks,
CHU Zhaowei

-----Original Message-----
From: Scott Dutton scott@exussum.co.uk
Sent: Wednesday, June 19, 2019 3:24 PM
To: internals@lists.php.net
Subject: [PHP-DEV] [RFC] [Vote] Base convert changes

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements

Two votes, one to raise a deprecated error in PHP7.4 (raised to exception in PHP

when base_convert encounters something it doesnt know how to convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--
To unsubscribe, visit:
http://www.php.net/unsub.php

5 years ago by Scott Dutton — view source

unread

Hi Chu,

Its currently coded to treat that as 'ff' so any odd amount of '-'
makes it negative, and any even amount is positive.

Thanks

Scott

Hi Scott,

Could you clarify what will happen if multiple negative sign occurs.
I didn't find it in the RFC.
e.g. base_convert('--ff', 16, 10)

Thanks,
CHU Zhaowei

-----Original Message-----
From: Scott Dutton scott@exussum.co.uk
Sent: Wednesday, June 19, 2019 3:24 PM
To: internals@lists.php.net
Subject: [PHP-DEV] [RFC] [Vote] Base convert changes

Hi all

I have put my RFC base convert changes to vote this morning

https://wiki.php.net/rfc/base_convert_improvements

Two votes, one to raise a deprecated error in PHP7.4 (raised to
exception in PHP

when base_convert encounters something it doesnt know how to
convert.

Second vote is to allow negative numbers, eg base_convert('-FF', 16,

would return -255 (this returns 255 currently)

Voting ends 3rd July

Thanks

Scott

--
To unsubscribe,
visit:
http://www.php.net/unsub.php

5 years ago by Scott Dutton — view source

unread

Hi all

I have made a PR for 7.4 for just the ignoring invalid input. I will
open a similar one for PHP 8.

https://github.com/php/php-src/pull/4328

All tests have been fixed for linux, there is one failure for windows
which I am looking in to.

Voting ends in a few days time so last chance to vote. looks like the
negative number part is not going to pass so the PR has not been
updated.
Link to RFC if you need it -
https://wiki.php.net/rfc/base_convert_improvements

Any questions please let me know

Thanks

Scott

5 years ago by Nikita Popov — view source

unread

Hi all

I have made a PR for 7.4 for just the ignoring invalid input. I will
open a similar one for PHP 8.

https://github.com/php/php-src/pull/4328

All tests have been fixed for linux, there is one failure for windows
which I am looking in to.

Voting ends in a few days time so last chance to vote. looks like the
negative number part is not going to pass so the PR has not been
updated.
Link to RFC if you need it -
https://wiki.php.net/rfc/base_convert_improvements

Any questions please let me know

Thanks

Scott

A possibility to consider would to only support negative numbers in bindec,
octdec and hexdec. Allowing negative numbers wouldn't change any existing
behavior (assuming we consider the leading "-" an error before, as we would
per this RFC). The problem is only on the decbin, decoct and dechex side,
where we currently treat the input as unsigned. base_convert could also
support negative numbers, though it might need some implementation changes
(it would be great to make base_convert directly work from string -> string
to avoid the float precision loss.)

Nikita