JDK-8336586 : Remove WritableElement and hide writing details of BufWriter
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.lang.classfile
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 24
  • Submitted: 2024-07-16
  • Updated: 2024-07-23
  • Resolved: 2024-07-23
Related Reports
CSR :  
Relates :  
Description
Summary
-------

Remove `java.lang.classfile.WritableElement` and remove some implementation detail methods related to `BufWriter`.

Problem
-------

`WritableElement` is an implementation detail of ClassFile API instead of part of the model. Also, `BufWriter` is only public for the purpose of writing user's `CustomAttribute`, but its functionalites way excced those requirements.

Solution
--------

Remove `WritableElement`; previous implementations are also no longer `ClassFileElement` except `Attribute` for compatibility purpose. Remove the methods `writeBytes(BufWriter)`, `writeList`, `writeListIndices`, `copyTo` from `BufWriter`, `compare` from `ClassReader`, and `writeTo` on `LocalVariable` and `LocalVariableType`.

This removes much unnecessary API surface, also makes it easier for users to understand ClassFile API.

Note that we currently determine that users don't need functionality to access what's already written in a BufWriter. We may add it back if we deem that necessary, but addition on demand is always better than the cost of maintaining a useless API.

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

    diff --git a/src/java.base/share/classes/java/lang/classfile/Annotation.java b/src/java.base/share/classes/java/lang/classfile/Annotation.java
    index 3e7548d0859..28c3672bf91 100644
    --- a/src/java.base/share/classes/java/lang/classfile/Annotation.java
    +++ b/src/java.base/share/classes/java/lang/classfile/Annotation.java
    @@ -51,7 +51,6 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface Annotation
    -        extends WritableElement<Annotation>
             permits TypeAnnotation, AnnotationImpl {
     
         /**
    diff --git a/src/java.base/share/classes/java/lang/classfile/AnnotationElement.java b/src/java.base/share/classes/java/lang/classfile/AnnotationElement.java
    index 41acb18e788..80adb07ec4b 100644
    --- a/src/java.base/share/classes/java/lang/classfile/AnnotationElement.java
    +++ b/src/java.base/share/classes/java/lang/classfile/AnnotationElement.java
    @@ -41,7 +41,6 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface AnnotationElement
    -        extends WritableElement<AnnotationElement>
             permits AnnotationImpl.AnnotationElementImpl {
     
         /**
    diff --git a/src/java.base/share/classes/java/lang/classfile/AnnotationValue.java b/src/java.base/share/classes/java/lang/classfile/AnnotationValue.java
    index 2882296b6bc..919c8a0441d 100644
    --- a/src/java.base/share/classes/java/lang/classfile/AnnotationValue.java
    +++ b/src/java.base/share/classes/java/lang/classfile/AnnotationValue.java
    @@ -49,7 +49,7 @@
      * @since 22
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
    -public sealed interface AnnotationValue extends WritableElement<AnnotationValue>
    +public sealed interface AnnotationValue
             permits AnnotationValue.OfAnnotation, AnnotationValue.OfArray,
                     AnnotationValue.OfConstant, AnnotationValue.OfClass,
                     AnnotationValue.OfEnum {
    diff --git a/src/java.base/share/classes/java/lang/classfile/Attribute.java b/src/java.base/share/classes/java/lang/classfile/Attribute.java
    index ad67bdf5365..718f164e8ef 100644
    --- a/src/java.base/share/classes/java/lang/classfile/Attribute.java
    +++ b/src/java.base/share/classes/java/lang/classfile/Attribute.java
    @@ -80,7 +80,7 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface Attribute<A extends Attribute<A>>
    -        extends WritableElement<A>
    +        extends ClassFileElement
             permits AnnotationDefaultAttribute, BootstrapMethodsAttribute,
                     CharacterRangeTableAttribute, CodeAttribute, CompilationIDAttribute,
                     ConstantValueAttribute, DeprecatedAttribute, EnclosingMethodAttribute,
    diff --git a/src/java.base/share/classes/java/lang/classfile/BootstrapMethodEntry.java b/src/java.base/share/classes/java/lang/classfile/BootstrapMethodEntry.java
    index c472dca8530..5ff3c449fe9 100644
    --- a/src/java.base/share/classes/java/lang/classfile/BootstrapMethodEntry.java
    +++ b/src/java.base/share/classes/java/lang/classfile/BootstrapMethodEntry.java
    @@ -43,7 +43,6 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface BootstrapMethodEntry
    -        extends WritableElement<BootstrapMethodEntry>
             permits BootstrapMethodEntryImpl {
     
         /**
    diff --git a/src/java.base/share/classes/java/lang/classfile/BufWriter.java b/src/java.base/share/classes/java/lang/classfile/BufWriter.java
    index 11d6a4501f0..c71b44e7c02 100644
    --- a/src/java.base/share/classes/java/lang/classfile/BufWriter.java
    +++ b/src/java.base/share/classes/java/lang/classfile/BufWriter.java
    @@ -110,13 +108,6 @@ public sealed interface BufWriter
          */
         void writeBytes(byte[] arr);
     
    -    /**
    -     * Write the contents of another {@link BufWriter} to the buffer
    -     *
    -     * @param other the other {@linkplain BufWriter}
    -     */
    -    void writeBytes(BufWriter other);
    -
         /**
          * Write a range of a byte array to the buffer
          *
    @@ -166,38 +157,8 @@ public sealed interface BufWriter
          */
         void writeIndexOrZero(PoolEntry entry);
     
    -    /**
    -     * Write a list of entities to the buffer.  The length of the list is
    -     * written as a {@code u2}, followed by the bytes corresponding to each
    -     * element in the list.  Writing of the entities is delegated to the entry.
    -     *
    -     * @param list the entities
    -     * @param <T> the type of entity
    -     */
    -    <T extends WritableElement<?>> void writeList(List<T> list);
    -
    -    /**
    -     * Write a list of constant pool entry indexes to the buffer.  The length
    -     * of the list is written as a {@code u2}, followed by a {@code u2} for each
    -     * entry in the list.
    -     *
    -     * @param list the list of entries
    -     * @throws IllegalArgumentException if any entry has invalid index
    -     */
    -    void writeListIndices(List<? extends PoolEntry> list);
    -
         /**
          * {@return the number of bytes that have been written to the buffer}
          */
         int size();
    -
    -    /**
    -     * Copy the contents of the buffer into a byte array.
    -     *
    -     * @param array the byte array
    -     * @param bufferOffset the offset into the array at which to write the
    -     *                     contents of the buffer
    -     * @throws IndexOutOfBoundsException if copying outside of the array bounds
    -     */
    -    void copyTo(byte[] array, int bufferOffset);
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/ClassFileElement.java b/src/java.base/share/classes/java/lang/classfile/ClassFileElement.java
    index 6452f52e042..a4a8203038f 100644
    --- a/src/java.base/share/classes/java/lang/classfile/ClassFileElement.java
    +++ b/src/java.base/share/classes/java/lang/classfile/ClassFileElement.java
    @@ -39,6 +39,6 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface ClassFileElement
    -        permits AttributedElement, CompoundElement, WritableElement,
    +        permits AttributedElement, CompoundElement, Attribute,
                     ClassElement, CodeElement, FieldElement, MethodElement {
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/ClassReader.java b/src/java.base/share/classes/java/lang/classfile/ClassReader.java
    index ef4a36729e6..735aae444fc 100644
    --- a/src/java.base/share/classes/java/lang/classfile/ClassReader.java
    +++ b/src/java.base/share/classes/java/lang/classfile/ClassReader.java
    @@ -189,19 +189,4 @@ public sealed interface ClassReader extends ConstantPool
          * @param len the length of the range
          */
         void copyBytesTo(BufWriter buf, int offset, int len);
    -
    -    /**
    -     * Compare a range of bytes from the classfile to a range of bytes within
    -     * a {@link BufWriter}.
    -     *
    -     * @param bufWriter the {@linkplain BufWriter}
    -     * @param bufWriterOffset the offset within the {@linkplain BufWriter}
    -     * @param classReaderOffset the offset within the classfile
    -     * @param length the length of the range
    -     * @return whether the two ranges were identical
    -     */
    -    boolean compare(BufWriter bufWriter,
    -                    int bufWriterOffset,
    -                    int classReaderOffset,
    -                    int length);
     }
    diff --git a/src/java.base/share/classes/java/lang/classfile/CustomAttribute.java b/src/java.base/share/classes/java/lang/classfile/CustomAttribute.java
    index 31544a0ba92..9fe492dc22c 100644
    --- a/src/java.base/share/classes/java/lang/classfile/CustomAttribute.java
    +++ b/src/java.base/share/classes/java/lang/classfile/CustomAttribute.java
    @@ -59,12 +59,6 @@ public final String attributeName() {
             return mapper.name();
         }
     
    -    @Override
    -    @SuppressWarnings("unchecked")
    -    public final void writeTo(BufWriter buf) {
    -        mapper.writeAttribute(buf, (T) this);
    -    }
    -
         @Override
         public String toString() {
             return String.format("CustomAttribute[name=%s]", mapper.name());
    diff --git a/src/java.base/share/classes/java/lang/classfile/FieldModel.java b/src/java.base/share/classes/java/lang/classfile/FieldModel.java
    index 35465dfe97d..e14f264ca2a 100644
    --- a/src/java.base/share/classes/java/lang/classfile/FieldModel.java
    +++ b/src/java.base/share/classes/java/lang/classfile/FieldModel.java
    @@ -42,7 +42,7 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface FieldModel
    -        extends WritableElement<FieldModel>, CompoundElement<FieldElement>, AttributedElement, ClassElement
    +        extends CompoundElement<FieldElement>, AttributedElement, ClassElement
             permits BufferedFieldBuilder.Model, FieldImpl {
     
         /** {@return the access flags} */
    diff --git a/src/java.base/share/classes/java/lang/classfile/MethodModel.java b/src/java.base/share/classes/java/lang/classfile/MethodModel.java
    index 3d91683e218..651bc194ee3 100644
    --- a/src/java.base/share/classes/java/lang/classfile/MethodModel.java
    +++ b/src/java.base/share/classes/java/lang/classfile/MethodModel.java
    @@ -42,7 +42,7 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface MethodModel
    -        extends WritableElement<MethodModel>, CompoundElement<MethodElement>, AttributedElement, ClassElement
    +        extends CompoundElement<MethodElement>, AttributedElement, ClassElement
             permits BufferedMethodBuilder.Model, MethodImpl {
     
         /** {@return the access flags} */
    diff --git a/src/java.base/share/classes/java/lang/classfile/WritableElement.java b/src/java.base/share/classes/java/lang/classfile/WritableElement.java
    deleted file mode 100644
    index 9a4db4e370f..00000000000
    --- a/src/java.base/share/classes/java/lang/classfile/WritableElement.java
    +++ /dev/null
    @@ -1,53 +0,0 @@
    -/*
    - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
    - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
    - *
    - * This code is free software; you can redistribute it and/or modify it
    - * under the terms of the GNU General Public License version 2 only, as
    - * published by the Free Software Foundation.  Oracle designates this
    - * particular file as subject to the "Classpath" exception as provided
    - * by Oracle in the LICENSE file that accompanied this code.
    - *
    - * This code is distributed in the hope that it will be useful, but WITHOUT
    - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    - * version 2 for more details (a copy is included in the LICENSE file that
    - * accompanied this code).
    - *
    - * You should have received a copy of the GNU General Public License version
    - * 2 along with this work; if not, write to the Free Software Foundation,
    - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    - *
    - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    - * or visit www.oracle.com if you need additional information or have any
    - * questions.
    - */
    -package java.lang.classfile;
    -
    -import java.lang.classfile.constantpool.ConstantPoolBuilder;
    -import java.lang.classfile.constantpool.PoolEntry;
    -import jdk.internal.classfile.impl.DirectFieldBuilder;
    -import jdk.internal.classfile.impl.DirectMethodBuilder;
    -import jdk.internal.javac.PreviewFeature;
    -
    -/**
    - * A classfile element that can encode itself as a stream of bytes in the
    - * encoding expected by the classfile format.
    - *
    - * @param <T> the type of the entity
    - *
    - * @sealedGraph
    - * @since 22
    - */
    -@PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
    -public sealed interface WritableElement<T> extends ClassFileElement
    -        permits Annotation, AnnotationElement, AnnotationValue, Attribute,
    -                PoolEntry, BootstrapMethodEntry, FieldModel, MethodModel,
    -                ConstantPoolBuilder, DirectFieldBuilder, DirectMethodBuilder {
    -    /**
    -     * Writes the element to the specified writer
    -     *
    -     * @param buf the writer
    -     */
    -    void writeTo(BufWriter buf);
    -}
    diff --git a/src/java.base/share/classes/java/lang/classfile/constantpool/ConstantPoolBuilder.java b/src/java.base/share/classes/java/lang/classfile/constantpool/ConstantPoolBuilder.java
    index a43e6f102ed..c0927175476 100644
    --- a/src/java.base/share/classes/java/lang/classfile/constantpool/ConstantPoolBuilder.java
    +++ b/src/java.base/share/classes/java/lang/classfile/constantpool/ConstantPoolBuilder.java
    @@ -61,7 +59,7 @@
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
     public sealed interface ConstantPoolBuilder
    -        extends ConstantPool, WritableElement<ConstantPool>
    +        extends ConstantPool
             permits SplitConstantPool, TemporaryConstantPool {
     
         /**
    diff --git a/src/java.base/share/classes/java/lang/classfile/constantpool/PoolEntry.java b/src/java.base/share/classes/java/lang/classfile/constantpool/PoolEntry.java
    index 49bd978e56a..c8b74a13928 100644
    --- a/src/java.base/share/classes/java/lang/classfile/constantpool/PoolEntry.java
    +++ b/src/java.base/share/classes/java/lang/classfile/constantpool/PoolEntry.java
    @@ -34,7 +33,7 @@
      * @since 22
      */
     @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API)
    -public sealed interface PoolEntry extends WritableElement<PoolEntry>
    +public sealed interface PoolEntry
             permits AnnotationConstantValueEntry, DynamicConstantPoolEntry,
                     LoadableConstantEntry, MemberRefEntry, ModuleEntry, NameAndTypeEntry,
                     PackageEntry {
    diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariable.java b/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariable.java
    index cd37ab94992..4d3d7553867 100644
    --- a/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariable.java
    +++ b/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariable.java
    @@ -84,14 +83,6 @@ default ClassDesc typeSymbol() {
          */
         Label endScope();
     
    -    /**
    -     * Writes the local variable to the specified writer
    -     *
    -     * @param buf the writer
    -     * @return true if the variable has been written
    -     */
    -    boolean writeTo(BufWriter buf);
    -
         /**
          * {@return a local variable pseudo-instruction}
          *
    diff --git a/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariableType.java b/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariableType.java
    index 1415d0bb57c..4409abe6db2 100644
    --- a/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariableType.java
    +++ b/src/java.base/share/classes/java/lang/classfile/instruction/LocalVariableType.java
    @@ -81,14 +81,6 @@ default Signature signatureSymbol() {
          */
         Label endScope();
     
    -    /**
    -     * Writes the local variable to the specified writer
    -     *
    -     * @param buf the writer
    -     * @return true if the variable has been written
    -     */
    -    boolean writeTo(BufWriter buf);
    -
         /**
          * {@return a local variable type pseudo-instruction}
          *
    
Comments
Moving to Approved.
23-07-2024

Very nice cleanup!
17-07-2024