<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<font size="4"><font face="monospace">Here's the combinator I had in
mind:<br>
<br>
static<T extends CodeElement> CodeTransform
statefulReplace(Class<T> target, <br>
Function<T, T> replacer, <br>
Supplier<T> supplier) { <br>
return new CodeTransform() {<br>
boolean found = false;<br>
<br>
@Override<br>
public void accept(CodeBuilder builder, CodeElement
element) {<br>
if (target.isAssignableFrom(element.getClass()))
{<br>
found = true;<br>
builder.accept(replacer.apply((T) element));<br>
}<br>
}<br>
<br>
@Override<br>
public void atEnd(CodeBuilder builder) {<br>
if (!found)<br>
builder.accept(supplier.get());<br>
}<br>
};<br>
} <br>
<br>
So then you could say<br>
<br>
CodeTransform = statefulReplace(RVAA.class, <br>
rvaa -> RVAA.of(...), <br>
() -> RVAA.of(...));<br>
<br>
It's cute, but I don't think it carries its weight in the API.
I think the real gap here is one of documentation, not
abstraction; finding the low-energy-state solution is not
obvious enough. Once you find the solution, making it
incrementally smaller with a combinator (or ten) is not as
important. <br>
<br>
Public Service Announcement: people think that code is the
important contribution, but sometimes doc is a more valuable
contribution. A "transform writers guide" would be an awesome
contribution. <br>
<br>
</font></font><br>
<div class="moz-cite-prefix">On 3/22/2023 8:42 AM, Dan Heidinga
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAJq4Gi51kpeJSGb5crBTmyMrFOX_sj7ibv_NZtBV21sFcy8BVQ@mail.gmail.com">
<div dir="ltr">
<div dir="ltr"><br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Mar 22, 2023 at
8:19 AM Brian Goetz <<a href="mailto:brian.goetz@oracle.com" moz-do-not-send="true" class="moz-txt-link-freetext">brian.goetz@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div> <font size="4"><font face="monospace">Before we get
into what API changes are needed, let's sync on the
problem first. <br>
<br>
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? <br>
<br>
2. Adding an annotation is complicated, and we
explored a number of solutions before settling in the
current place. There are three cases:<br>
<br>
- There is no RVAA, so you have to add one<br>
- There is an RVAA, but it already has your
annotation, so there's nothing to do <br>
- There is an RVAA, and it doesn't have your
annotation, so you have to transform it<br>
<br>
(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?)<br>
<br>
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.)<br>
<br>
<a href="https://mail.openjdk.org/pipermail/classfile-api-dev/2022-August/000108.html" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://mail.openjdk.org/pipermail/classfile-api-dev/2022-August/000108.html</a><br>
<br>
Here's the recommended code for that situation:<br>
<br>
() -> new ClassTransform() {<br>
boolean foundNM = false;<br>
<br>
void accept(ClassBuilder b, ClassElement e) {<br>
switch (e) {<br>
case NestMembersAttribute a -> {
transform a; foundNM = <br>
true; }<br>
... other transform stuff ...<br>
}<br>
}<br>
<br>
void atEnd(ClassBuilder b) {<br>
if (!foundNM)<br>
builder.accept(NestMembersAttribute.of(...));<br>
}<br>
}<br>
<br>
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.<br>
</font></font></div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>TLDR: Brian's proposed code is the best you can do for
this.</div>
<div><br>
</div>
<div>--Dan</div>
<div><br>
</div>
<div>[0] <a href="https://urldefense.com/v3/__https://github.com/DanHeidinga/jdk-sandbox/blob/9c5bf15522e76ebc106a433758c87623dec4827b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateLambdaClassesPlugin.java*L238-L385__;Iw!!ACWV5N9M2RV99hQ!J-O_EVx6bVisxcY4VatFAgX4IqHCqDFZv9lj0hkNJQT9QCbPXVxQCCzwbWh-hHP8ZbPTFRJrET9BugBPlJc$" moz-do-not-send="true">https://github.com/DanHeidinga/jdk-sandbox/blob/9c5bf15522e76ebc106a433758c87623dec4827b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateLambdaClassesPlugin.java#L238-L385</a></div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div><font size="4"><font face="monospace"> <br>
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. <br>
<br>
<br>
<br>
</font></font><br>
<div>On 3/21/2023 11:38 PM, - wrote:<br>
</div>
<blockquote type="cite">
<pre>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
</pre>
</blockquote>
<br>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</body>
</html>