On Fri, 26 May 2023 14:56:57 GMT, Adam Sotona <asot...@openjdk.org> wrote:
> Classfile context object and multi-state options have been discussed at > https://mail.openjdk.org/pipermail/classfile-api-dev/2023-May/000321.html > This patch implements the proposed changes in Classfile API and fixes all > affected code across JDK sources and tests. > > Please review. > > Thanks, > Adam A side comment about the API changes: I think we should recommend users to store Classfile instances in static final variables if sharing is reasonable, like in Module and BindingSpecializer; this way, we can cache class hierarchy resolution information across builds. src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 87: > 85: * @param r a function mapping attribute names to attribute mappers > 86: */ > 87: record AttributeMapperOption(Function<Utf8Entry, AttributeMapper<?>> > attributeMapper) implements Option { We might want to convert these options into interfaces and move the implementation records into ClassfileImpl, like Brian suggested about ClassHierarchyInfo. src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 208: > 206: */ > 207: default ClassModel parse(byte[] bytes) { > 208: return new ClassImpl(bytes, (ClassfileImpl)this); Should we implement this in ClassfileImpl to avoid the redundant cast? src/java.base/share/classes/jdk/internal/classfile/components/ClassRemapper.java line 102: > 100: * @return re-mapped class file bytes > 101: */ > 102: default byte[] remapClass(ClassModel clm) { This api should ask for a Classfile object instead. Thankfully, the new Classfile object actually allows us to find potentially bugs in our code (say if the remapped class refer to declared types outside of system class loader) ------------- PR Review: https://git.openjdk.org/jdk/pull/14180#pullrequestreview-1452244549 PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1211113876 PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1211114779 PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1211116312
