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