Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document str_increment() and str_decrement() #2797

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Sep 26, 2023

Part of #2796

@Girgias Girgias force-pushed the doc-inc-dec-str-function branch 2 times, most recently from f94acd5 to f199a26 Compare September 26, 2023 19:59
Copy link
Member

@saundefined saundefined left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Comment on lines 55 to 56
A <classname>ValueError</classname> is thrown if
<parameter>string</parameter> is out of the decrement range.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is that? At "0"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is that? At "0"?

Seems to At "1".
ValueError is thrown if we try to decrement "1".

<?php

$a = 'a5';
for ($i = 0; $i < 1000; $i++) {
    var_dump("decrementing $a");
    $a = str_decrement($a);
    var_dump($a);
}
sapi/cli/php test.php
string(15) "decrementing a1"
string(2) "a0"
string(15) "decrementing a0"
string(1) "9"
string(14) "decrementing 9"
string(1) "8"
string(14) "decrementing 8"
string(1) "7"
string(14) "decrementing 7"
string(1) "6"
string(14) "decrementing 6"
string(1) "5"
string(14) "decrementing 5"
string(1) "4"
string(14) "decrementing 4"
string(1) "3"
string(14) "decrementing 3"
string(1) "2"
string(14) "decrementing 2"
string(1) "1"
string(14) "decrementing 1"

Fatal error: Uncaught ValueError: str_decrement(): Argument #1 ($string) "1" is out of decrement range in /home/mumumu/build/php-src/test.php:6

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better to describe the decrement range by example. @Girgias

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might need to ask @iluuu1994 as he changed the decrement function after merging the RFC PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, "1" erroring is a bug. The fix is here: php/php-src#12339

This error occurs when there's a remaining carry after all digits have been decremented. This can occur for string like "000", "0a", etc. The leading digit is also removed if it is a 0 (e.g. when going from "10" to "09"). However, I have not considered the case of "1" going to "0", with only a single digit, which does not need to be removed.

@Girgias Girgias merged commit 57c8357 into php:master Oct 6, 2023
2 checks passed
@Girgias Girgias deleted the doc-inc-dec-str-function branch October 6, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants