JDK-8336354 : Remove ClassFile XxxBuilder.original and change XxxModel.parent behavior
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.lang.classfile
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 24
  • Submitted: 2024-07-15
  • Updated: 2024-07-23
  • Resolved: 2024-07-23
Related Reports
CSR :  
Relates :  
Description
Summary
-------

Remove `Optional<XxxModel> original()` from `ClassBuilder`, `MethodBuilder`, `FieldBuilder`, `CodeBuilder`. Make `Optional<XxxModel> parent()` return present if and only if the model is an immutable read model (instead of a wip transforming model)

Problem
-------

The `original` method in the builders are not useful. They are useful only to transforms, but most ad-hoc transforms go through `transformXxx` which has access to the original model. The `ClassFileBuilder::transform` has the same functionality of transforming an "original" model with a transform, but the called builder does not have an "original" model as a result, further increasing the confusion.

The `parent()` method allows easy access to the parents of models, such as methods or fields from a class model. However, it currently returns the original class model for a transformed buffered field or method (but not code), which is erroneous as those models can't be found in the class by iteration.

Solution
--------

Remove the `original()` methods; they have no usages. Make the `parent()` methods only return present value if the model is bound and not partially transformed ("buffered"), that it can be discovered on the parent model by compound element component iteration.

Specification
-------------

No specification change for `parent()`. For `original()`:

    diff --git a/src/java.base/share/classes/java/lang/classfile/ClassBuilder.java b/src/java.base/share/classes/java/lang/classfile/ClassBuilder.java
    index f1b8bb13d27..a55b93e7c07 100644
    --- a/src/java.base/share/classes/java/lang/classfile/ClassBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/ClassBuilder.java
    @@ -58,12 +57,6 @@ public sealed interface ClassBuilder
             extends ClassFileBuilder<ClassElement, ClassBuilder>
             permits ChainedClassBuilder, DirectClassBuilder {
     
    -    /**
    -     * {@return the {@link ClassModel} representing the class being transformed,
    -     * if this class builder represents the transformation of some {@link ClassModel}}
    -     */
    -    Optional<ClassModel> original();
    -
         /**
          * Sets the classfile version.
          * @param major the major version number
    diff --git a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
    index a1666375091..a6ec41cc437 100644
    --- a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
    @@ -127,12 +127,6 @@ public sealed interface CodeBuilder
             extends ClassFileBuilder<CodeElement, CodeBuilder>
             permits CodeBuilder.BlockCodeBuilder, ChainedCodeBuilder, TerminalCodeBuilder, NonterminalCodeBuilder {
     
    -    /**
    -     * {@return the {@link CodeModel} representing the method body being transformed,
    -     * if this code builder represents the transformation of some {@link CodeModel}}
    -     */
    -    Optional<CodeModel> original();
    -
         /** {@return a fresh unbound label} */
         Label newLabel();
     
    diff --git a/src/java.base/share/classes/java/lang/classfile/FieldBuilder.java b/src/java.base/share/classes/java/lang/classfile/FieldBuilder.java
    index 18a598398ec..c0e733676f7 100644
    --- a/src/java.base/share/classes/java/lang/classfile/FieldBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/FieldBuilder.java
    @@ -68,9 +67,4 @@ default FieldBuilder withFlags(AccessFlag... flags) {
             return with(AccessFlags.ofField(flags));
         }
     
    -    /**
    -     * {@return the {@link FieldModel} representing the field being transformed,
    -     * if this field builder represents the transformation of some {@link FieldModel}}
    -     */
    -    Optional<FieldModel> original();
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/MethodBuilder.java b/src/java.base/share/classes/java/lang/classfile/MethodBuilder.java
    index ece292c361b..4cf34c95ed0 100644
    --- a/src/java.base/share/classes/java/lang/classfile/MethodBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/MethodBuilder.java
    @@ -50,12 +49,6 @@ public sealed interface MethodBuilder
             extends ClassFileBuilder<MethodElement, MethodBuilder>
             permits ChainedMethodBuilder, TerminalMethodBuilder {
     
    -    /**
    -     * {@return the {@link MethodModel} representing the method being transformed,
    -     * if this method builder represents the transformation of some {@link MethodModel}}
    -     */
    -    Optional<MethodModel> original();
    -
         /**
          * Sets the method access flags.
          * @param flags the access flags, as a bit mask


Comments
Moving to Approved.
23-07-2024