Planned features of a classfile context object

[Adam Sotona]

From: Brian Goetz <brian.goetz at oracle.com>
Date: Friday, 26 May 2023 23:16
To: Adam Sotona <adam.sotona at oracle.com>, liangchenblue at gmail.com <liangchenblue at gmail.com>, classfile-api-dev <classfile-api-dev at openjdk.org>
Subject: Re: Planned features of a classfile context object
Some review comments on your patch:

 - I'd prefer for all of the static methods in Classfile to go away, though I can imagine choosing to leave the most basic version (no options) of build/parse around as conveniences.  If we leave these, the Javadoc should make it clear that these are equivalent to Context.of().parse(...).
 - I am OK with calling the context class "Classfile" for the time being; this makes the diff easier to follow as it makes it clear that the static methods are being promoted to instance methods, and we can discuss name later.
OK, Classfile.of().parse(…) is much easier to find than Classfile.Context.of().parse(…), so I’ll remove the static methods.

 - I see you cache the Options in BufWriterImpl/BufferedCodeBuilder/etc, but we should probably be caching the full context (impl) object instead, and fetching options out of the context.
impl.Options was the context class and now it only become implementation of Classfile.Context, I’ll rename it to ClassfileImpl.

 - (Syntax nit) Some enum options are fully descriptive of what they are (DROP_DEBUG_ELEMENTS), and some are implicit (ALWAYS_GENERATE -- generate what?).  I think I would prefer to err on the side of fully descriptive (and people can use static imports to drop the qualifier if they like), but we should be consistent.  There should be a convention of what the default is (like "first is always default") for options that are enums.

I’ll fix it, thanks.
Adam

On 5/26/2023 11:18 AM, Adam Sotona wrote:
This is a raw prototype of Classfile.Context implementation and the new options:
https://github.com/openjdk/jdk/pull/14180/files
https://github.com/openjdk/jdk/blob/4456ca78bc67138930dc313f71aba58ef1cf5f13/src/java.base/share/classes/jdk/internal/classfile/Classfile.java

It is far from finished, however before going further I would appreciate your comments (mainly on options naming).

Thanks,
Adam


From: Brian Goetz <brian.goetz at oracle.com><mailto:brian.goetz at oracle.com>
Date: Thursday, 25 May 2023 20:31
To: Adam Sotona <adam.sotona at oracle.com><mailto:adam.sotona at oracle.com>, liangchenblue at gmail.com<mailto:liangchenblue at gmail.com> <liangchenblue at gmail.com><mailto:liangchenblue at gmail.com>, classfile-api-dev <classfile-api-dev at openjdk.org><mailto:classfile-api-dev at openjdk.org>
Subject: Re: Planned features of a classfile context object
A model created within a context can retain a reference to its creating context.  (We do a lot of this sort of thing already, where MethodModel retains a reference to ClassModel, and where CPBuilder retains a reference to Options.)  So for operations that require a context, a model derived from a classfile can use the context that created it.

Methods like parse/build/transform will be moved to the classfile context object.
On 5/25/2023 2:02 PM, Adam Sotona wrote:
I’m trying to figure out how the context will be passed down to the models (if it suppose to be detached from the models).
Do we plan to enforce another CC argument to all models methods requiring context?
Or do we plan to strip down all context-sensitive methods from all models and attach them to a kind of context-wrappers?
For example codeModel.forEach(Classfile.Context.of(…), …) or Classfile.Context.of(…).forEach(codeModel, …) ?

Thanks,
Adam


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230529/2093e45b/attachment.htm>