Re: RFR: 8332597: Remove redundant methods from j.l.classfile.ClassReader API [v2]
On Fri, 24 May 2024 16:41:33 GMT, Adam Sotona wrote: >> j.l.classfile.ClassReader instance is exposed in the Class-File API through >> j.l.classfile.AttributeMapper::readAttribute method only. >> ClassReader only purpose is to serve as a tool for reading content of a >> custom attribute in a user-provided AttribtueMapper. >> It contains useful set of low-level class reading methods for user to >> implement a custom attribute content parser. >> >> However methods ClassReader::thisClassPos and >> ClassReader::skipAttributeHolder are not necessary for a custom attribute >> content parsing and so redundant in the API. >> Class-File API implementation internally use these methods, however they >> should not be exposed in the API. >> >> This patch removes the methods from the API. >> >> Please review. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Merge branch 'master' into JDK-8332597-classreader-redundancy > ># Conflicts: ># src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java > - 8332597: Remove redundant methods from j.l.classfile.ClassReader API @asotona Can you transition the CSR to proposed again for reviewing? - PR Comment: https://git.openjdk.org/jdk/pull/19323#issuecomment-2136470497
Re: RFR: 8332597: Remove redundant methods from j.l.classfile.ClassReader API [v2]
On Fri, 24 May 2024 16:41:33 GMT, Adam Sotona wrote: >> j.l.classfile.ClassReader instance is exposed in the Class-File API through >> j.l.classfile.AttributeMapper::readAttribute method only. >> ClassReader only purpose is to serve as a tool for reading content of a >> custom attribute in a user-provided AttribtueMapper. >> It contains useful set of low-level class reading methods for user to >> implement a custom attribute content parser. >> >> However methods ClassReader::thisClassPos and >> ClassReader::skipAttributeHolder are not necessary for a custom attribute >> content parsing and so redundant in the API. >> Class-File API implementation internally use these methods, however they >> should not be exposed in the API. >> >> This patch removes the methods from the API. >> >> Please review. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Merge branch 'master' into JDK-8332597-classreader-redundancy > ># Conflicts: ># src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java > - 8332597: Remove redundant methods from j.l.classfile.ClassReader API Marked as reviewed by jlahoda (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19323#pullrequestreview-2082655142
Re: RFR: 8332597: Remove redundant methods from j.l.classfile.ClassReader API [v2]
> j.l.classfile.ClassReader instance is exposed in the Class-File API through > j.l.classfile.AttributeMapper::readAttribute method only. > ClassReader only purpose is to serve as a tool for reading content of a > custom attribute in a user-provided AttribtueMapper. > It contains useful set of low-level class reading methods for user to > implement a custom attribute content parser. > > However methods ClassReader::thisClassPos and > ClassReader::skipAttributeHolder are not necessary for a custom attribute > content parsing and so redundant in the API. > Class-File API implementation internally use these methods, however they > should not be exposed in the API. > > This patch removes the methods from the API. > > Please review. > > Thanks, > Adam Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Merge branch 'master' into JDK-8332597-classreader-redundancy # Conflicts: #src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java - 8332597: Remove redundant methods from j.l.classfile.ClassReader API - Changes: https://git.openjdk.org/jdk/pull/19323/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19323&range=01 Stats: 23 lines in 5 files changed: 0 ins; 17 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/19323.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19323/head:pull/19323 PR: https://git.openjdk.org/jdk/pull/19323
