Skip to content

Add warning and convert to exception in string offset assignment: #5063

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

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jan 7, 2020

Convert the empty string assignment to an Error as per RFC [1]
Add a warning that only the first byte will be assigned to the offset if provided
a needle that is longer than one byte.

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

As said per #5013

Convert the empty string assignment to an Error as per RFC [1]
Add a warning that only the first byte will be assigned to the offset if provided
a needle that is longer than one byte.

[1] https://wiki.php.net/rfc/engine_warnings
@nikic
Copy link
Member

nikic commented Jan 7, 2020

Looks basically good, but likely needs a JIT change as well.

@Girgias
Copy link
Member Author

Girgias commented Jan 7, 2020

Looks basically good, but likely needs a JIT change as well.

Isn't that what the changes in ext/opcache/jit/zend_jit_helpers.c do? change the JIT? Or is there somewhere else that I need to look into?

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Oops, I missed that. LGTM.

@php-pulls php-pulls merged commit bfe3f93 into php:master Jan 7, 2020
@Girgias Girgias deleted the error-on-variable-string-offset branch January 7, 2020 21:57
@carusogabriel carusogabriel modified the milestone: PHP 8.0 May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants