Revisit j.l.classfile.CodeBuilder API surface

[Brian Goetz]

Let me fill in some history here.

Much of the design process for this library could be described as an 
iterated game of "find the primitive."  The first version of the API had 
a zillion ad-hoc methods, with little structural relation.  At some 
point we hit upon a model-driven analysis, and broke down instructions 
into families (loads, stores, etc), and at that point, we thought 
methods like loadInstruction were the primitives.  But we hadn't 
actually hit bottom yet; after many rounds of refactoring, it became 
clear that there was one primitive for the builder hierarchy, now called 
`with`, and all of the builder methods now bottom out at that.

At the same time, we added a number of "convenience" methods such as 
aload_0() and aload(int n) (which now just delegate to with() or 
loadInstruction()), and over time, we found that most generative use 
cases used these methods much more than the primitives (which is fine.)

At this point, I think many of the xxxInstruction (and its rigid naming 
convention) are vestiges of a previous attempt to organize the API.  
Some still have uses, though should be renamed to reflect that they are 
mere conveniences (such as arrayLoadInstruction to arrayLoad).

(Does anyone use operator(opcode), or can we drop that one too?)

So I support this simplification.

Having done the refactor, were there any surprises in the usages of 
various CodeBuilder methods?


On 1/5/2024 10:38 AM, Adam Sotona wrote:
>
> Hi,
>
> j.l.classfile.CodeBuilder API consist of more than 230 methods.
>
> Existing ClassFile API use cases proved the concept of one big 
> CodeBuilder is comfortable. However there are some redundancies, 
> glitches in the naming, some frequently used methods are hard to find 
> and some methods have low practical use.
>
> Majorityof the methods may be dividedinto three main levels:
> 1.methods building low level bytecode instructions according to JVMS 
> chapter 6.5 (aaload, aastore, aconst_null...)
> 2.methods reconstructing individual subtypes of 
> j.l.classfile.Instruction from given arguments (loadInstruction, 
> storeInstruction, incrementInstruction, branchInstruction...)
> 3.methods modeling high level code blocks (block, ifThen, ifThenElse, 
> trying...)
>
> Many methods from level 2 (with suffix `Instruction`) seem to be 
> obsolete or misplaced. Some of them are duplicates of methods from 
> level 1, some are obsolete and some are very useful, however a bit 
> hidden. The API should be cleaned a bit while in preview.
>
> I would like to open a discussion on the following proposed changes in 
> the CodeBuilder methods:
>
>   * incrementInstruction remove as duplicate of iinc
>   * lookupSwitchInstruction remove as duplicate of lookupswitch
>   * tableSwitchInstruction remove as duplicate of tableswitch
>   * throwInstruction remove as duplicate of athrow
>   * invokeDynamicInstruction remove as duplicate of invokedynamic
>   * stackInstruction remove as obsolete with suggested replacements:
>     with(StackInstruction.of(...))
>   * monitorInstruction remove as obsolete with suggested replacements:
>     monitorenter, monitorexit, or with(MonitorInstruction.of(...))
>   * nopInstruction remove as duplicate of nop
>   * typecheckInstruction remove as obsolete with suggested
>     replacements: checkcast, instanceOf, or
>     with(TypeCheckInstruction.of(...))
>   * loadInstruction rename to loadLocal
>   * storeInstruction rename to storeLocal
>   * branchInstruction rename to branch
>   * invokeInstruction rename to invoke
>   * newObjectInstruction rename to newObject
>   * newPrimitiveArrayInstruction rename to newPrimitiveArray
>   * newReferenceArrayInstruction rename to newReferenceArray
>   * newMultidimensionalArrayInstruction rename to newMultidimensionalArray
>   * arrayLoadInstruction rename to arrayLoad
>   * arrayStoreInstruction rename to arrayStore
>   * convertInstruction rename to conversion
>   * operatorInstruction rename to operator
>   * constantInstruction rename to loadConstant
>   * fieldInstruction rename to fieldAccess
>   * instanceof_ rename to instanceOf
>   * returnInstruction rename to return_
>
> Here is the related RFE: https://bugs.openjdk.org/browse/JDK-8323058
>
> Draft of the CSR (no spec yet): 
> https://bugs.openjdk.org/browse/JDK-8323067
>
> And draft of the Pull Request: https://github.com/openjdk/jdk/pull/17282
>
> Any comments are welcome.
>
> Thank you,
>
> Adam
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20240105/60cd9cf0/attachment-0001.htm>