RFR: 8304846: Provide a shared utility to dump generated classes defined via Lookup API [v5]

[Mandy Chung]

On Mon, 3 Apr 2023 18:07:52 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Mandy Chung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   set -D<key> or -D<key>=true will enable dumping
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java line 85:
> 
>> 83: 
>> 84:     // Used to ensure that dumped class files for failed definitions have a unique class name
>> 85:     private static final AtomicInteger counter = new AtomicInteger();
> 
> This counter could be removed now, it looks like.

thanks for catching it. will fix.

> src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java line 115:
> 
>> 113:         if (dumper.isEnabled() && !path.equals(dumper.dumpPath())) {
>> 114:             throw new IllegalArgumentException("mismatched dump path for " + key);
>> 115:         }
> 
> I don't see how this exception case could ever occur, given that `dumper.dumpPath` is directly coming from `dir`, and `validateDumpDir` doesn't return a modified path either.
> 
> Can this check be removed?

This is to check if two callers get the `ClassFileDumper` of the same key but different path.


  ClassFileDumper.getInstance("jdk.foo.dump", "DUMP_DIR");
  ClassFileDumper.getInstance("jdk.foo.dump", "DUMP_NEW_DIR");  <--- IAE

> src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java line 145:
> 
>> 143:     public Path pathname(String internalName) {
>> 144:         return dumpDir.resolve(encodeForFilename(internalName) + ".class");
>> 145:     }
> 
> Could be `private` I think

It's public in case the caller who invokes `ClassDefiner::defineClass` wants to print the path to which the classfile is written.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13182#discussion_r1156367767
PR Review Comment: https://git.openjdk.org/jdk/pull/13182#discussion_r1156366699
PR Review Comment: https://git.openjdk.org/jdk/pull/13182#discussion_r1156367573