Skip to content

Move constants from dir.c to stub file #13312

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 7 commits into from
Feb 13, 2024

Conversation

haszi
Copy link
Contributor

@haszi haszi commented Feb 2, 2024

Moves registering some constant from dir.c to its corresponding stub file with four constant registrations remaining in dir.c.

@kocsismate pinging you as this is a stub file change as well.

@kocsismate
Copy link
Member

Very nice catch, I'm not sure why I didn't put these constants into stub! Thanks for the PR!

@haszi
Copy link
Contributor Author

haszi commented Feb 6, 2024

Very nice catch, I'm not sure why I didn't put these constants into stub! Thanks for the PR!

There are a few more of these that I think could be moved to a stub file. I'll open a PR for these today or tomorrow.
Additionally, there are ~20-25 constants whose values are evaluated during runtime. It might be a hack but if gen_stub.php could generate separate functions for these (as opposed to putting them in e.g. register_dir_symbols with the rest of the constants) then all constants could easily be moved to stub files.

I used the below command to list all the remaining constants not registered in stub files.
grep -rE --exclude=*.{php,md} --exclude={*arginfo.h,zend_constants.h} 'REGISTER_(.+)_CONSTANT'

Copy link
Member

@kocsismate kocsismate left a comment

Choose a reason for hiding this comment

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

LGTM, I had only 2 small nits. I'll merge the PR once these are resolved ) Thanks for your work!

@haszi haszi requested a review from kocsismate February 12, 2024 09:21
@haszi haszi requested a review from kocsismate February 13, 2024 07:31
@kocsismate kocsismate merged commit e957e25 into php:master Feb 13, 2024
@haszi haszi deleted the Move-dir-constants-to-stub-file branch February 14, 2024 09:15
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.

3 participants