Skip to content
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

Zend/tests: merge constant_expressions into constexpr, update names #17872

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

DanielEScherzer
Copy link
Contributor

While reviewing the existing tests in the constexpr directory, I found that some of the names were not updated to reflect the contents when the contents were changed in #9301.

Follow-up to #15638

While reviewing the existing tests in the `constexpr` directory, I found that
some of the names were not updated to reflect the contents when the contents
were changed in php#9301.

Follow-up to php#15638
@DanielEScherzer
Copy link
Contributor Author

CC @TimWolla who caught the duplicate directories

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

I leave the actual review to @Girgias who I believe has reviewed the previous ones.

Invalid operation in new arg in const expr
Valid operation in new arg in const expr
Copy link
Member

Choose a reason for hiding this comment

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

This one is interesting, because AFAICT it no longer is a constexpr context since https://wiki.php.net/rfc/arbitrary_static_variable_initializers

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Seems sensible

@Girgias Girgias merged commit 7b8a61a into php:master Feb 21, 2025
9 checks passed
@DanielEScherzer DanielEScherzer deleted the tests-constexpr branch February 21, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants