Skip to content

Allow arbitrary const expressions in backed enums #8190

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 1 commit into from

Conversation

iluuu1994
Copy link
Member

@iluuu1994 iluuu1994 commented Mar 11, 2022

Closes GH-7821

Open issues:

  • Every other run or so, ZSTR_HAS_CE_CACHE(class_name) returns true for "self" even though zend_alloc_ce_cache specifically guards for that. Sounds like memory corruption but I couldn't figure out where yet
  • We probably need more forgiving error handling than zend_error_noreturn now that the errors are moved to runtime. All other errors in zend_do_link_class seem to be fatal so I think this is consistent.
  • I changed the enum_backing_table to store the case directly but that might not actually work for internal enums (or we change zend_enum_add_case not to modify the backing table anymore and move that to after the constants have been initialized)
  • Add tests for Can't use inherited constant value via 'self' in enum case value #7821 (comment)
  • Can we add the backed_enum_table to the inheritance cache to avoid re-population on every request? That would require tracking dependencies of all classes in all expressions so likely not feasible.
Siteproxy
@bwoebi
Copy link
Member

bwoebi commented Apr 5, 2022

Assigning a static property a constant expr of the wrong type gets you a TypeError (thus should be identical here). Everything else probably should just throw an Error.

In case zend_update_class_constants fails, you do nothing. This takes care of its own error handling deep down. Just return a failure. (E.g. zend_ast_evaluate directly throws exceptions.) Just ensure that the enum backed table building happens before the class is marked as linked. (i.e. move the zend_enum_build_backed_enum_table call up.)

Copy link
Member Author

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

@nikic The backing table is now built when first accessing a constant or when first calling from/tryFrom. Let me know if you think this is acceptable.

Verified

This commit was signed with the committer’s verified signature.
iluuu1994 Ilija Tovilo
Closes phpGH-7821
Closes phpGH-8190
Closes phpGH-8418
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.

Can't use inherited constant value via 'self' in enum case value
6 participants