ext/standard: Deprecate passing string which are not one byte long to ord() #19440
Conversation
Girgias
force-pushed
the
8.5-dep-ord
branch
from
d558365 to
7006002
Compare
|
@nielsdos do you have any idea why the CI is failling with a memory exhaustion? I can't seem to reproduce this locally at all :| |
It happens during output collection. Could it be that one test emits an extreme amount of deprecations? |
|
You could try removing the |
Girgias
requested review from
alexdowad,
bukka and
youkidearitai
as code owners
| php_error_docref(NULL, E_DEPRECATED, | ||
| "Providing a string which is not one byte long is deprecated, use ord($str[0]) instead"); | ||
| } | ||
| } |
There was a problem hiding this comment.
what happens if the deprecation is converted to an exception - should RETURN_LONG still be used? Can you add a test case for that?
There was a problem hiding this comment.
In case of a conversion, the return value is simply discarded by the VM or JIT. So no special exception handling is needed
ext/standard/string.c
Outdated
| "Providing an empty string is deprecated"); | ||
| } else { | ||
| php_error_docref(NULL, E_DEPRECATED, | ||
| "Providing a string which is not one byte long is deprecated, use ord($str[0]) instead"); |
There was a problem hiding this comment.
I think this is the restrictive vs non-restrictive clause thing, so it needs to be "that" instead of "which" ?
| "Providing a string which is not one byte long is deprecated, use ord($str[0]) instead"); | |
| "Providing a string that is not one byte long is deprecated, use ord($str[0]) instead"); |
There was a problem hiding this comment.
@Crell any opinion on the wording?
There was a problem hiding this comment.
This is a restrictive clause, so that is slightly more correct but both are widely used. I'd say go with that.
https://www.merriam-webster.com/grammar/when-to-use-that-and-which
The bigger issue is that the comma should be a period, and use should start a new sentence. Otherwise it's a run-on sentence.
Girgias
force-pushed
the
8.5-dep-ord
branch
from
d7c8d95 to
000fe44
Compare
This PR was merged into the 6.4 branch. Discussion ---------- [Console] do not pass the empty string to ord() | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | | License | MIT see php/php-src#19440 and https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_passing_string_which_are_not_one_byte_long_to_ord Commits ------- 30037a1 do not pass the empty string to ord()
This PR was merged into the 7.3 branch. Discussion ---------- [JsonPath] do not pass more than one byte to ord() | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT see php/php-src#19440 Commits ------- cbdcffc do not pass more than one byte to ord()
8 participants
RFC: https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_passing_string_which_are_not_one_byte_long_to_ord