API suggestions

[Dan Heidinga]

On Wed, Mar 22, 2023 at 8:19 AM Brian Goetz <brian.goetz at oracle.com> wrote:

> Before we get into what API changes are needed, let's sync on the problem
> first.
>
> 1.  CodeTransform has an `atStart` method just as it has an `atEnd`
> method, and I presume overriding `atStart(builder)` is adequate to your
> need.  Is the issue simply that there is no canned combinator for
> startHandler as there is for endHandler?
>
> 2.  Adding an annotation is complicated, and we explored a number of
> solutions before settling in the current place.  There are three cases:
>
>  - There is no RVAA, so you have to add one
>  - There is an RVAA, but it already has your annotation, so there's
> nothing to do
>  - There is an RVAA, and it doesn't have your annotation, so you have to
> transform it
>
> (Actually there is a fourth case: it has your annotation, but with
> _different properties_; there is @Foo(3) but you want to add @Foo(4).  Now
> what do you do?  Replace it?  Leave it?  Add both?)
>
> The trickiness is compounded by the fact that the latter two cases can be
> handled by transforming the element for RVAA, but the first case can't be
> handled until the end handler, and you have to remember whether you saw an
> RVAA or not.  So the process is inherently stateful.  Some options were
> covered in this thread (which talked about NestMembers, but the story is
> the same for annotations.)
>
>
> https://mail.openjdk.org/pipermail/classfile-api-dev/2022-August/000108.html
>
> Here's the recommended code for that situation:
>
>      () -> new ClassTransform() {
>          boolean foundNM = false;
>
>          void accept(ClassBuilder b, ClassElement e) {
>              switch (e) {
>                  case NestMembersAttribute a -> { transform a; foundNM =
> true; }
>                  ... other transform stuff ...
>              }
>          }
>
>          void atEnd(ClassBuilder b) {
>              if (!foundNM)
>                  builder.accept(NestMembersAttribute.of(...));
>          }
>      }
>
> It's irritating because you have to do three things -- keep track of some
> state, rewrite an attribute if you see it, and generate an attribute if you
> don't, but it's really not so lengthy or hard to follow.
>

Here's a link to my various attempts to find a better way to express this
[0] when exploring NestMember attribute changes.  Wrapping Brian's
suggested pattern in a named class makes it a little easier to read at the
point of use but there's no single pattern for this (that I've found) that
results in significantly cleaner code.

TLDR: Brian's proposed code is the best you can do for this.

--Dan

[0]
https://github.com/DanHeidinga/jdk-sandbox/blob/9c5bf15522e76ebc106a433758c87623dec4827b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateLambdaClassesPlugin.java#L238-L385


>
> I can imagine capturing this in a combinator, where you provide two
> lambdas, one a transform on your favorite attribute, and one a supplier of
> that attribute, but I don't think it will be that much easier to use, it
> just automate managing and acting on the boolean.
>
>
>
>
> On 3/21/2023 11:38 PM, - wrote:
>
> Hi,
> After housekeeping the few tests after the latest integration of
> additional constants in ConstantDescs, I have found two potentially
> common patterns in bytecode manipulation and wish to suggest new APIs:
>
> 1. Injection of code in the beginning of methods
> Currently, our CodeTransform only has an endHandler; however, this is
> less useful, for a method can have multiple exit points. Usually to
> track method calls, bytecode manipulation, such as those in jfr and
> instrumentation, inject at the beginning of methods, which is
> guaranteed to be on the code path.
>
> Thus, I suggest a `startHandler(Consumer<CodeBuilder>)` for
> CodeTransform for this purpose, instead of having to explicitly
> override atStart(CodeBuilder) in non-lambda CodeTransform.
>
> 2. Adding new annotations
> In ASM, annotation injection is done simply via extra annotation
> visitor calls, which makes it compatible for either single or multiple
> annotations. In contrast, in the Classfile API, if we want to add an
> annotation, feeding a new RuntimeVisibleAnnotationAttribute will
> destroy the previous RuntimeVisibleAnnotationAttribute, and handling
> via a stateful ClassTransform is a bit too lengthy for users.
>
> I don't know if we should have an API like Map::computeIfAbsent for
> attributes or if we will simplify it via other means.
>
> Chen Liang
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230322/4284046b/attachment-0001.htm>