On Fri, 2 Aug 2024 07:57:27 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> Chen Liang has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Remove generics, further improve docs >> - Merge branch 'master' of https://github.com/openjdk/jdk into >> fix/annotation-constant >> - 8335927: Revisit AnnotationConstantValueEntry and >> AnnotationValue.OfConstant > > src/java.base/share/classes/java/lang/classfile/AnnotationValue.java line 90: > >> 88: */ >> 89: @PreviewFeature(feature = PreviewFeature.Feature.CLASSFILE_API) >> 90: sealed interface OfConstant<C extends AnnotationConstantValueEntry, >> R extends Comparable<R> & Constable> > > I suggest to minimize use of generics in combination with sealed interface > trees and pattern matching. It does not give many benefits, it takes out a > part of the code readability and adds many unnecessary brackets and > questionmarks: > > case AnnotationValue.OfConstant<?, ?> oc -> > > > `<R extends Comparable<R> & Constable>` at least brings a benefit of types > combination. However I still think it does not outweigh the generics use > cost. None of the generics parameter is near to orthogonal to the > `OfConstant` nor critical for the API to operate. Removed the type parameters. Please review again. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20436#discussion_r1701848599