Skip to content

DNF types bug fixes and others #11961

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 5 commits into from
Closed

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 14, 2023

This is a draft and a bit of a mishmash of solving bugs and adding tests for issues I discovered while working on #11884

I've added a function to verify that a zend_type is actually well-defined, which seems to be the true underlying issue of #11884 as for some reason something is happening during the variance checks that messes up the "inner" intersection type list allocation variant (so going from arena to something else) when the outside union type list is still as expected arena allocated.

I can't find any "obvious" issues with the persistence size calculation, nor the actual persistence operation in opcache.

@Girgias Girgias force-pushed the internal-list-types branch from bfd227d to 2094880 Compare August 14, 2023 21:03
@ju1ius
Copy link
Contributor

ju1ius commented Aug 14, 2023

You might also add validation logic to zend_declare_typed_class_constant? 😉

@Girgias
Copy link
Member Author

Girgias commented Aug 14, 2023

You might also add validation logic to zend_declare_typed_class_constant? wink

Good point, but the PR for 8.2 I'm writing won't have the validation logic, and typed constants is only a thing since 8.3

@Girgias
Copy link
Member Author

Girgias commented Sep 8, 2023

Most of this PR was merged as 02a80c5 when fixing all the other DNF bugs.

The only useful thing from here would be the zend_type validation code for debug builds, but not sure how worthwhile this is.

@Girgias Girgias closed this Sep 8, 2023
@Girgias Girgias deleted the internal-list-types branch September 8, 2023 13:30
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