RFR: 8311077: Fix -Wconversion warnings in jvmti code [v3]

[Matias Saavedra Silva]

On Thu, 29 Jun 2023 17:29:50 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Please review change for mostly fixing return types in the constant pool and metadata to fix -Wconversion warnings in JVMTI code.  The order of preference for changes are: 1. change the types to more distinct types (fields in the constant pool are u2 because that's their size in the classfile), 2. add direct int casts if the value has been checked in asserts above, and 3. checked_cast<> if not verified, and 4. added some pointer_delta_as_ints where needed.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add a comment about u1 cast to new_index for the ldc bytecode.

Looks good, thanks for this change! I listed a few considerations below, but if you don't think they are necessary, what you have is just fine.

src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 2195:

> 2193:       case Bytecodes::_ldc:
> 2194:       {
> 2195:         u1 cp_index = *(bcp + 1);

Constant pool indices are usually u2, why does this need to be a u1?

src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 2268:

> 2266:       {
> 2267:         address p = bcp + 1;
> 2268:         int cp_index = Bytes::get_Java_u2(p);

Should this field also be a u2?

src/hotspot/share/prims/methodComparator.cpp line 79:

> 77:   case Bytecodes::_instanceof     : {
> 78:     int cpi_old = s_old->get_index_u2();
> 79:     int cpi_new = s_new->get_index_u2();

These constant pool accessors like `klass_at_noresolve` currently take in `int which` but I think it's worth looking at if this is necessary. Constant pool indices and constant pool cache indices seem to both be u2 so it might be a better option to change the arguments to u2 here to avoid the need to cast.

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

Marked as reviewed by matsaave (Committer).

PR Review: https://git.openjdk.org/jdk/pull/14710#pullrequestreview-1505776921
PR Review Comment: https://git.openjdk.org/jdk/pull/14710#discussion_r1246928389
PR Review Comment: https://git.openjdk.org/jdk/pull/14710#discussion_r1246927933
PR Review Comment: https://git.openjdk.org/jdk/pull/14710#discussion_r1246934498