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

Coleen Phillimore coleenp at openjdk.org
Thu Jun 29 19:57:59 UTC 2023


On Thu, 29 Jun 2023 17:23:44 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:
>> 
>>   Add a comment about u1 cast to new_index for the ldc bytecode.
>
> 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?

This could be a u2 to avoid confusion.  Since it's ldc, the cp_index in the ldc bytecode is only a u1 but this didn't get a Wconversion error so I should probably keep it as int.

> 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?

It could be a u2, but since find_new_index's parameter is int and didn't need a cast further down, I didn't change it to one.

> 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.

I had to change these two lines because BytecodeStream::get_index_u2 returns an int, so got the warning and this didn't need to be declared with u2.  get_index_u2() could be fixed to return a u2 but I didn't want to go that far as no casts were involved in this change.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14710#discussion_r1247101683
PR Review Comment: https://git.openjdk.org/jdk/pull/14710#discussion_r1247069649
PR Review Comment: https://git.openjdk.org/jdk/pull/14710#discussion_r1247053822


More information about the hotspot-dev mailing list