Skip to content

Add some const qualifiers in zend_string/hash #8304

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 2 commits into from
Apr 20, 2022

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Apr 5, 2022

I was confused as to why most of the APIs did not have a const modifier until I actually added them and got reminded that zend_string_hash_func() needs the zend_string to be mutable, so not sure how worthwhile these changes actually are.

@Girgias Girgias force-pushed the hash-const-qualifiers branch from 8289d72 to 13d7700 Compare April 7, 2022 13:04
Copy link
Contributor

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

These changes look good to me. I have a small suggestion to cast the result of zend_string_hash_val to void. This is a convention that some linters understand that says "I'm knowingly not using the result here." I've found it generally to be useful. What do you think?

@Girgias Girgias merged commit c2547ab into php:master Apr 20, 2022
@Girgias Girgias deleted the hash-const-qualifiers branch April 20, 2022 14:56
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.

2 participants