On Mon, 2 Oct 2023 08:34:19 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> Classfile API is an internal library under package `jdk.internal.classfile` >> in JDK 21. >> This pull request turns the Classfile API into a preview feature and moves >> it into `java.lang.classfile`. >> It repackages all uses across JDK and tests and adds lots of missing Javadoc. >> >> This PR goes in sync with >> [JDK-8308754](https://bugs.openjdk.org/browse/JDK-8308754): Class-File API >> (Preview) (CSR) >> and [JDK-8280389](https://bugs.openjdk.org/browse/JDK-8280389): Class-File >> API (Preview) (JEP). >> >> Online javadoc is available at: >> https://cr.openjdk.org/~asotona/JDK-8308753-preview/api/java.base/java/lang/classfile/package-summary.html >> >> In addition to the primary transition to preview, this pull request also >> includes: >> - All Classfile* classes ranamed to ClassFile* (based on JEP discussion). >> - A new preview feature, `CLASSFILE_API`, has been added. >> - Buildsystem tool required a little patch to support annotated modules. >> - All JDK modules using the Classfile API are newly participating in the >> preview. >> - All tests that use the Classfile API now have preview enabled. >> - A few Javac tests not allowing preview have been partially reverted; their >> conversion can be re-applied when the Classfile API leaves preview mode. >> >> Despite the number of affected files, this pull request is relatively >> straight-forward. The preview version of the Classfile API is based on the >> internal version of the library from the JDK master branch, and there are no >> API features added. >> >> Please review this pull request to help the Classfile API turn into a >> preview. >> >> Any comments are welcome. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > fixed javadoc typo src/java.base/share/classes/java/lang/classfile/ClassTransform.java line 164: > 162: > 163: /** > 164: * @implSpec Should these overridden method have `@inheritDoc` ? If not, their javadoc won't show much, I believe. (same might be true for other methods) src/java.base/share/classes/java/lang/classfile/CodeBuilder.java line 596: > 594: * @param type the object type > 595: * @return this builder > 596: * @throws IllegalArgumentException if {@code type} represents a > primitive type What about arrays? (ClassDesc has an `isArray` predicate too). This probably applies to other methods too. src/java.base/share/classes/java/lang/classfile/CodeTransform.java line 101: > 99: > 100: /** > 101: * @implSpec The default implementation returns a resolved transform; `... returns a resolved transform that is bound to the given code builder` or `... returns a resolved transform bound to the given code builder` src/java.base/share/classes/java/lang/classfile/FieldTransform.java line 116: > 114: > 115: /** > 116: * @implSpec The default implementation returns a resolved transform > with all Perhaps lose the `all its parts` - not sure that makes the comment any clearer? src/java.base/share/classes/java/lang/classfile/MethodSignature.java line 59: > 57: > 58: /** > 59: * {@return method signature for a raw (no generic information) > method descriptor} Isn't this missing an article - e.g. `return the method signature` or `return a method signature` ? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342685585 PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342691511 PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342693515 PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342694834 PR Review Comment: https://git.openjdk.org/jdk/pull/15706#discussion_r1342696367