Skip to content

Fix property_exists() for XMLReader #16079

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 3 commits into from
Sep 28, 2024

Conversation

kocsismate
Copy link
Member

No description provided.

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Good find, thanks! I guess I assumed this handler exists.
Similarly it seems that an unset handler is missing... I kinda forgot I applied virtual properties here too.
This should target PHP-8.4 btw

return 0;
} else {
zval_ptr_dtor(&rv);
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

This should respect the rules for isset and empty, so for empty this should call zend_is_true for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yeah, I wanted to copy-paste the code I wrote for DatePeriod, but somehow I got distracted... I'm fixing it right now.

@kocsismate
Copy link
Member Author

kocsismate commented Sep 27, 2024

This should target PHP-8.4 btw

Yeah, I based my branch on PHP-8.4, but I always keep forgetting to set the right target when it comes to creating the PR.

@kocsismate kocsismate changed the base branch from master to PHP-8.4 September 27, 2024 06:13
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Found one minor mistake

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Thank you!

@kocsismate kocsismate merged commit f4f2fe5 into php:PHP-8.4 Sep 28, 2024
10 checks passed
kocsismate added a commit that referenced this pull request Sep 28, 2024
* PHP-8.4:
  Fix property_exists() and unset() for XMLReader (#16079)
@kocsismate kocsismate deleted the xmlreader-has-property branch September 28, 2024 19:09
jorgsowa pushed a commit to jorgsowa/php-src that referenced this pull request Oct 1, 2024
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.

2 participants