Skip to content

Conversation

@withinboredom
Copy link
Member

This PR adjusts extgen to handle:

  • the case where the go code already contains an init() function.
  • support for //export_php:module init to handle resource creation during module startup
  • support for //export_php:module shutdown to handle resource cleanup during module shutdown

@withinboredom withinboredom changed the title Add support for init/minit/mshutdown extgen: Add support for init/minit/mshutdown Aug 3, 2025
@withinboredom withinboredom requested a review from dunglas August 3, 2025 19:54
@withinboredom withinboredom marked this pull request as ready for review August 4, 2025 05:49
@withinboredom
Copy link
Member Author

My last commit breaks classes, but is needed for functions-only extensions. I'll need to make that import conditional on whether or not there are classes present and add a test for it.

@withinboredom
Copy link
Member Author

This one is probably ready for review now @dunglas

@alexandre-daubois
Copy link
Member

This is really nice and certainly useful. Thanks!

Bikeshedding: should we call the directives //export_php:minit and mshutdown to match PHP's macros? Your proposition also suits me fine. It's just to see if someone would prefer this 🙂

Copy link
Member

@alexandre-daubois alexandre-daubois left a comment

Choose a reason for hiding this comment

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

I didn't see a test for this: what happens if these new directives are called multiple times in the same file?

If you want an example, we already limited //export_php:namespace to one per file

Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Signed-off-by: Robert Landers <landers.robert@gmail.com>
Copy link
Member

@alexandre-daubois alexandre-daubois left a comment

Choose a reason for hiding this comment

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

Works for me, thanks for adding the duplicated directive checks!

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.

4 participants