Skip to content

Improve class entry generation #6701

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 3 commits into from

Conversation

kocsismate
Copy link
Member

No description provided.

@kocsismate
Copy link
Member Author

kocsismate commented Feb 15, 2021

After making the functions static, the compiler could detect that register_class_ZendTestNS_Foo() and register_class_ZendTestNS_Foo2() are defined but not used. I think it should be a supported use-case not to use these functions, shouldn't it? Is there any possibility to circumvent the issue?

@nikic
Copy link
Member

nikic commented Feb 15, 2021

@kocsismate I don't think that should be a supported use-case. Why declare classes in the stub if they don't actually get declared?

@kocsismate
Copy link
Member Author

@nikic After thinking about it a little bit, I realized that you are right. My initial idea was that the reason of not using a class entry generator function could be that it is missing a needed feature (e.g. gen_stub.php doesn't yet support generating properties with a union type of multiple classes, or with constant default values).

However, my argument was bad, since the missing features can be fixed up later manually (e.g. by declaring the missing properties after the ce registration). That said, I'll fix the issue by declaring the missing classes. :)

@kocsismate
Copy link
Member Author

I'm currently working on adding support for generating properties with an union type of multiple classes.

@nikic
Copy link
Member

nikic commented Feb 16, 2021

Can you please land the first two commits to reduce the diff?

php-pulls pushed a commit that referenced this pull request Feb 16, 2021
@php-pulls php-pulls closed this in 803779e Feb 16, 2021
@kocsismate kocsismate deleted the stub-ce-followup branch February 16, 2021 14:57
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