Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8336027

Hide Transform implementation for Class-File API

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Approved
    • Icon: P4 P4
    • 24
    • core-libs
    • None
    • source
    • minimal
    • Changes to preview APIs have minimal compatibility risk.
    • Java API
    • SE

      Summary

      Remove ClassFileTransform.ResolvedTransform, ClassFileTransform::resolve, and ClassFileBuilder::canWriteDirect. Update ClassFileBuilder::transform to return itself for chaining.

      Problem

      The ResolvedTransform exposed is error-prone as users may miss to call the start or end handler. Users also have not enough resources to implement resolve, as the ChainedBuilder are only available to ClassFile API implementations. These APIs together are confusing to users who wish to transform class files. Also, ClassFileBuilder does not need to write constant pool indices, so it doesn't need canWriteDirect checks; only attribute mappers do. And there's no reason why ClassFileBuilder::transform does not support chaining calls like other builder methods.

      Solution

      Remove these confusing APIs. Change ClassFileBuilder::transform to return the builder itself. Users wishing to use transform are advised to use ClassFileBuilder::transform, ClassBuilder::transformMethod, etc., and chaining transform will happen through ClassFileTransform::andThen.

      An API specification update is planned later, after ClassFile API has been cleaned up; otherwise, specification changes are prone to outdated information and merge conflicts.

      Specification

      diff --git a/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java b/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java
      index 9381c0510ef..46dcb1cf375 100644
      --- a/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java
      +++ b/src/java.base/share/classes/java/lang/classfile/ClassFileBuilder.java
      @@ -71,25 +72,18 @@ default void accept(E e) {
            */
           ConstantPoolBuilder constantPool();
      
      -    /**
      -     * {@return whether the provided constant pool is compatible with this builder}
      -     * @param source the constant pool to test compatibility with
      -     */
      -    default boolean canWriteDirect(ConstantPool source) {
      -        return constantPool().canWriteDirect(source);
      -    }
      -
           /**
            * Apply a transform to a model, directing results to this builder.
            * @param model the model to transform
            * @param transform the transform to apply
      +     * @return this builder
            */
      -    default void transform(CompoundElement<E> model, ClassFileTransform<?, E, B> transform) {
      +    default B transform(CompoundElement<E> model, ClassFileTransform<?, E, B> transform) {
               @SuppressWarnings("unchecked")
               B builder = (B) this;
      -        var resolved = transform.resolve(builder);
      +        var resolved = TransformImpl.resolve(transform, builder);
               resolved.startHandler().run();
               model.forEachElement(resolved.consumer());
               resolved.endHandler().run();
      +        return builder;
           }
       }
      diff --git a/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java b/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java
      index f67da06a36d..851f4deb03e 100644
      --- a/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java
      +++ b/src/java.base/share/classes/java/lang/classfile/ClassFileTransform.java
      @@ -124,46 +123,4 @@ default void atStart(B builder) {
            * @return the chained transform
            */
           C andThen(C next);
      -
      -    /**
      -     * The result of binding a transform to a builder.  Used primarily within
      -     * the implementation to perform transformation.
      -     *
      -     * @param <E> the element type
      -     *
      -     * @since 22
      -     */
      -    @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
      -    interface ResolvedTransform<E extends ClassFileElement> {
      -        /**
      -         * {@return a {@link Consumer} to receive elements}
      -         */
      -        Consumer<E> consumer();
      -
      -        /**
      -         * {@return an action to call at the end of transformation}
      -         */
      -        Runnable endHandler();
      -
      -        /**
      -         * {@return an action to call at the start of transformation}
      -         */
      -        Runnable startHandler();
      -    }
      -
      -    /**
      -     * Bind a transform to a builder.  If the transform is chained, intermediate
      -     * builders are created for each chain link.  If the transform is stateful
      -     * (see, e.g., {@link ClassTransform#ofStateful(Supplier)}), the supplier is
      -     * invoked to get a fresh transform object.
      -     *
      -     * <p>This method is a low-level method that should rarely be used by
      -     * user code; most of the time, user code should prefer
      -     * {@link ClassFileBuilder#transform(CompoundElement, ClassFileTransform)},
      -     * which resolves the transform and executes it on the current builder.
      -     *
      -     * @param builder the builder to bind to
      -     * @return the bound result
      -     */
      -    ResolvedTransform<E> resolve(B builder);
       }
      diff --git a/src/java.base/share/classes/java/lang/classfile/ClassTransform.java b/src/java.base/share/classes/java/lang/classfile/ClassTransform.java
      index a2391b320f9..743a3985114 100644
      --- a/src/java.base/share/classes/java/lang/classfile/ClassTransform.java
      +++ b/src/java.base/share/classes/java/lang/classfile/ClassTransform.java
      @@ -171,15 +171,4 @@ static ClassTransform transformingFields(FieldTransform xform) {
           default ClassTransform andThen(ClassTransform t) {
               return new TransformImpl.ChainedClassTransform(this, t);
           }
      -
      -    /**
      -     * @implSpec The default implementation returns a resolved transform bound
      -     *           to the given class builder.
      -     */
      -    @Override
      -    default ResolvedTransform<ClassElement> resolve(ClassBuilder builder) {
      -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
      -                                                         () -> atEnd(builder),
      -                                                         () -> atStart(builder));
      -    }
       }
      diff --git a/src/java.base/share/classes/java/lang/classfile/CodeTransform.java b/src/java.base/share/classes/java/lang/classfile/CodeTransform.java
      index a0996a9cb34..cdc7a3b1434 100644
      --- a/src/java.base/share/classes/java/lang/classfile/CodeTransform.java
      +++ b/src/java.base/share/classes/java/lang/classfile/CodeTransform.java
      @@ -96,15 +96,4 @@ public void atEnd(CodeBuilder builder) {
           default CodeTransform andThen(CodeTransform t) {
               return new TransformImpl.ChainedCodeTransform(this, t);
           }
      -
      -    /**
      -     * @implSpec The default implementation returns a resolved transform bound
      -     *           to the given code builder.
      -     */
      -    @Override
      -    default ResolvedTransform<CodeElement> resolve(CodeBuilder builder) {
      -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
      -                                                         () -> atEnd(builder),
      -                                                         () -> atStart(builder));
      -    }
       }
      diff --git a/src/java.base/share/classes/java/lang/classfile/FieldTransform.java b/src/java.base/share/classes/java/lang/classfile/FieldTransform.java
      index 1a27a1d6ee6..4e39f1e9c7f 100644
      --- a/src/java.base/share/classes/java/lang/classfile/FieldTransform.java
      +++ b/src/java.base/share/classes/java/lang/classfile/FieldTransform.java
      @@ -111,15 +111,4 @@ static FieldTransform dropping(Predicate<FieldElement> filter) {
           default FieldTransform andThen(FieldTransform t) {
               return new TransformImpl.ChainedFieldTransform(this, t);
           }
      -
      -    /**
      -     * @implSpec The default implementation returns a resolved transform bound
      -     *           to the given field builder.
      -     */
      -    @Override
      -    default ResolvedTransform<FieldElement> resolve(FieldBuilder builder) {
      -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
      -                                                         () -> atEnd(builder),
      -                                                         () -> atStart(builder));
      -    }
       }
      diff --git a/src/java.base/share/classes/java/lang/classfile/MethodTransform.java b/src/java.base/share/classes/java/lang/classfile/MethodTransform.java
      index 829ca041c5a..e7e024ebc34 100644
      --- a/src/java.base/share/classes/java/lang/classfile/MethodTransform.java
      +++ b/src/java.base/share/classes/java/lang/classfile/MethodTransform.java
      @@ -111,17 +111,6 @@ static MethodTransform transformingCode(CodeTransform xform) {
               return new TransformImpl.MethodCodeTransform(xform);
           }
      
      -    /**
      -     * @implSpec The default implementation returns a resolved transform bound
      -     *           to the given method builder.
      -     */
      -    @Override
      -    default ResolvedTransform<MethodElement> resolve(MethodBuilder builder) {
      -        return new TransformImpl.ResolvedTransformImpl<>(e -> accept(builder, e),
      -                                                         () -> atEnd(builder),
      -                                                         () -> atStart(builder));
      -    }
      -
           /**
            * @implSpec
            * The default implementation returns this method transform chained with another

            liach Chen Liang
            liach Chen Liang
            Adam Sotona
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: