On Wed, 24 May 2023 18:19:33 GMT, Coleen Phillimore <[email protected]> wrote:
>> This change uses a number of ways to eliminate -Wconversion warnings in the >> metadata files in the oops directory. >> >> 1. narrow return types to u2 if the accessor is for a field or value that's >> u2 (u2 is most common for constMethod fields and constant pool indices) >> 2. Use checked_cast<type> for places where we know the int value is u2 or s2 >> but propagating these types is too much fan out. >> 3. Use plain casts where it's obvious that the int value fits in the >> casted-to type. >> 4. Moved KlassKind to be contained in Klass to add the Unknown enum value to >> use instead of -1. >> 5. Moved the compute_from_signature function into ConstMethod as it sets >> values in ConstMethod and the parameters are changed in the set functions. >> Removed some pass through functions in Method. >> >> Tested with tier1-4. > > Coleen Phillimore has updated the pull request incrementally with one > additional commit since the last revision: > > Fred's comments. Looks good, I just had a few minor comments that might be unimportant. Otherwise, approved! 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? 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 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. ------------- Marked as reviewed by matsaave (Committer). PR Review: https://git.openjdk.org/jdk/pull/14092#pullrequestreview-1442597537 PR Review Comment: https://git.openjdk.org/jdk/pull/14092#discussion_r1204662039 PR Review Comment: https://git.openjdk.org/jdk/pull/14092#discussion_r1204662242 PR Review Comment: https://git.openjdk.org/jdk/pull/14092#discussion_r1204658756
