Skip to content

Don't check $this existence in object opcodes #3822

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

Closed
wants to merge 2 commits into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Feb 13, 2019

We are now guaranteed that $this always exists inside methods. Remove checks for $this existence from the individual object opcodes.

Instead make use of ZEND_FETCH_THIS in the cases where $this is used but isn't guaranteed to exist, such as closures and pseudo-main scope.

@nikic
Copy link
Member Author

nikic commented Feb 13, 2019

@dstogov I'm a bit concerned that this might make closures slower, because they will have less efficient $this fetching. I'm wondering if we could get away with forbidding $this unbinding for closures that use $this.

@dstogov
Copy link
Member

dstogov commented Feb 13, 2019

@nikic, I would agree.

This will affect only Closure::bind(), no Reflection?

May be, in addition, revisit zend_valid_closure_binding() and convert all warnings into exceptions...

We are now guaranteed that $this always exists inside methods.
Remove checks for $this existence from the individual object opcodes.

Instead make use of ZEND_FETCH_THIS in the cases where $this is used
but isn't guaranteed to exist, such as closures and pseudo-main scope.
@nikic
Copy link
Member Author

nikic commented Oct 28, 2019

@dstogov This should be ready to go now. The closure changes have been made, so we can now always assume $this exists in methods and closures (if it uses $this), and only have to use ZEND_FETCH_THIS in free-standing code (file included inside method for example).

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

Looks great!
It may make sense to implement optimization pass that replaces dominated FETCH_THIS by IS_UNDEDF.

@php-pulls php-pulls closed this in aef8836 Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants