RFR: 8308655: Narrow types of ConstantPool and ConstMethod returns [v2]

Coleen Phillimore coleenp at openjdk.org
Wed May 24 20:06:05 UTC 2023


On Wed, 24 May 2023 19:24:17 GMT, Matias Saavedra Silva <matsaave at openjdk.org> 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


More information about the serviceability-dev mailing list