Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v34]
On Fri, 24 Nov 2023 11:00:18 GMT, Alan Bateman wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove Test > > src/java.base/share/classes/java/lang/Class.java line 4797: > >> 4795: PublicMethods.MethodList res = getMethodsRecursive(name, >> parameterTypes, true, publicOnly); >> 4796: return res == null ? null : >> getReflectionFactory().copyMethod(res.getMostSpecific()); >> 4797: } > > Would you mind moving this up to follow getDeclaredPublicMethods, as these > are the two method finders exposed via JavaLangAccess. Updated > src/java.base/share/classes/jdk/internal/misc/MethodFinder.java line 40: > >> 38: } >> 39: >> 40: private static final JavaLangAccess JLA = >> SharedSecrets.getJavaLangAccess(); > > The new shared secret and usage looks fine. It would be good to add a short > class description and method description and make it clear it's for launcher > usage, we don't want this used for anything else. Personally I would move JLA > to to the top rather than after the constructor. Updated - PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1404324242 PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1404324338
Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v34]
On Thu, 23 Nov 2023 15:55:42 GMT, Jim Laskey wrote: >> Address changes from JEP 445 to JEP 463. >> >> - Move from a SYNTHETIC unnamed class to a MANDATED implicit class. >> >> - Don't mark class on read. >> >> - Remove reflection and annotation processing related to unnamed classes. >> >> - Simplify main method search. > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Test src/java.base/share/classes/jdk/internal/misc/MethodFinder.java line 40: > 38: } > 39: > 40: private static final JavaLangAccess JLA = > SharedSecrets.getJavaLangAccess(); The new shared secret and usage looks fine. It would be good to add a short class description and method description and make it clear it's for launcher usage, we don't want this used for anything else. Personally I would move JLA to to the top rather than after the constructor. - PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1404252179
Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v34]
On Thu, 23 Nov 2023 15:55:42 GMT, Jim Laskey wrote: >> Address changes from JEP 445 to JEP 463. >> >> - Move from a SYNTHETIC unnamed class to a MANDATED implicit class. >> >> - Don't mark class on read. >> >> - Remove reflection and annotation processing related to unnamed classes. >> >> - Simplify main method search. > > Jim Laskey has updated the pull request incrementally with one additional > commit since the last revision: > > Remove Test src/java.base/share/classes/java/lang/Class.java line 4797: > 4795: PublicMethods.MethodList res = getMethodsRecursive(name, > parameterTypes, true, publicOnly); > 4796: return res == null ? null : > getReflectionFactory().copyMethod(res.getMostSpecific()); > 4797: } Would you mind moving this up to follow getDeclaredPublicMethods, as these are the two are the method finders exposed via JavaLangAccess. - PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1404226792
Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v34]
> Address changes from JEP 445 to JEP 463. > > - Move from a SYNTHETIC unnamed class to a MANDATED implicit class. > > - Don't mark class on read. > > - Remove reflection and annotation processing related to unnamed classes. > > - Simplify main method search. Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Remove Test - Changes: - all: https://git.openjdk.org/jdk/pull/16461/files - new: https://git.openjdk.org/jdk/pull/16461/files/420f6b42..690dac1d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16461=33 - incr: https://webrevs.openjdk.org/?repo=jdk=16461=32-33 Stats: 131 lines in 1 file changed: 0 ins; 131 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16461.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16461/head:pull/16461 PR: https://git.openjdk.org/jdk/pull/16461
