Skip to content
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

Consider making GenerationContext extend from Closeable #28684

Closed
snicoll opened this issue Jun 23, 2022 · 2 comments
Closed

Consider making GenerationContext extend from Closeable #28684

snicoll opened this issue Jun 23, 2022 · 2 comments
Labels
status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement

Comments

@snicoll
Copy link
Member

snicoll commented Jun 23, 2022

The GenerationContext keeps generated classes around so that multiple processors (or multiple round of the same processor) get a chance to contribute to the same file if necessary.

The Default implementation has a writeGeneratedContent that must be called at the end. We thought that it might be more obvious to make the context extend from Closeable so that a method that does this must be implemented.

The problem is withName that creates copies of the context with specialized naming convention. We don't really want those to be closed as the "main" context is still being processed. It's also unclear if we want to prevent users to use the context once it has been closed.

@snicoll snicoll added status: waiting-for-triage An issue we've not yet triaged or decided on type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Jun 23, 2022
@snicoll
Copy link
Member Author

snicoll commented Jul 26, 2022

I've experimented with this and this is more noise than I thought it would be. The withName that gives you another instance isn't really Closeable as we expect the root would be closed at the end and flush all resources. That said, it is an interesting exercise as things are a bit undocumented at the moment and anyone could call writeGeneratedContext on a child (with a cast though) and that would write everything.

I think we'd be better off with some additional documentation.

@snicoll
Copy link
Member Author

snicoll commented Jul 26, 2022

This has been superseded by #28877

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
@snicoll snicoll added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant