Consistent return and notices for substr type of functions #5476
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Girgias
force-pushed
the
normalize-substr-oob-start
branch
2 times, most recently
from
5c41586 to
f7c4856
Compare
cmb69
reviewed
Apr 28, 2020
Girgias
force-pushed
the
normalize-substr-oob-start
branch
from
f7c4856 to
7b49892
Compare
|
@nikic should I add the notices for the length argument at the same time? And do I need to convert some of the ValueErrors back to notices? (as they were warnings before I don't know what would be the best approach) |
Girgias
force-pushed
the
normalize-substr-oob-start
branch
from
69919b5 to
ea31d46
Compare
Girgias
force-pushed
the
normalize-substr-oob-start
branch
2 times, most recently
from
1aafe97 to
621e084
Compare
Girgias
force-pushed
the
normalize-substr-oob-start
branch
from
621e084 to
416f81a
Compare
|
@kocsismate @nikic I've rebased this but haven't checked the code again as I don't know where to go with this. |
|
Closing in favour of #6182 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trying to make those return consistent but I'm not 100% sure @nikic can you have a look.
I've also took the luxury to remove a couple of variations tests which are more or less identical and generate massive diffs which have nul bytes in the output (which are a PITA).
Stubs are not yet update (nor Opcache if relevant).I hope I also covered all the functions which probably should have the same behaviour.
Extensions which have these sort of functions: