On Wed, 24 May 2023 19:24:17 GMT, Matias Saavedra Silva <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fred's comments. > > src/hotspot/share/classfile/classLoaderExt.cpp line 70: > >> 68: Arguments::assert_is_dumping_archive(); >> 69: int start_index = ClassLoader::num_boot_classpath_entries(); >> 70: _app_class_paths_start_index = checked_cast<jshort>(start_index); > > This might be a misunderstanding but are these meant to be s2 instead of > jshort? The declaration is jshort so I left it jshort to not modify the declaration, even though s2 is the same and better. > src/hotspot/share/classfile/classLoaderExt.cpp line 122: > >> 120: int start_index = ClassLoader::num_boot_classpath_entries() + >> 121: ClassLoader::num_app_classpath_entries(); >> 122: _app_module_paths_start_index = checked_cast<jshort>(start_index); > > Same question as above Same here. I didn't want it to not match the declaration. > src/hotspot/share/oops/klass.hpp line 79: > >> 77: TypeArrayKlassKind, >> 78: ObjArrayKlassKind, >> 79: Unknown > > Maybe this should be `UnknownKind` to be consistent with the other options. That's a great suggestion. I renamed the enum UnknownKlassKind to be like the others. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14092#discussion_r1204700040 PR Review Comment: https://git.openjdk.org/jdk/pull/14092#discussion_r1204700310 PR Review Comment: https://git.openjdk.org/jdk/pull/14092#discussion_r1204699802
