RFR: 8313785: Fix -Wconversion warnings in prims code [v2]

[Coleen Phillimore]

On Fri, 4 Aug 2023 21:11:05 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Dean's suggested changes.
>
> src/hotspot/share/prims/jni.cpp line 209:
> 
>> 207: 
>> 208: 
>> 209: intptr_t jfieldIDWorkaround::encode_klass_hash(Klass* k, intptr_t offset) {
> 
> The caller always passes an int, so why not change the parameter to int?
> Suggestion:
> 
> intptr_t jfieldIDWorkaround::encode_klass_hash(Klass* k, int offset) {

Ok.  That's a good suggestion.

> src/hotspot/share/prims/jni.cpp line 1906:
> 
>> 1904:     o = JvmtiExport::jni_SetField_probe(thread, obj, o, k, fieldID, false, SigType, (jvalue *)&field_value); \
>> 1905:   } \
>> 1906:   if (SigType == JVM_SIGNATURE_BOOLEAN) { value = (Argument)(((jboolean)value) & 1); } \
> 
> This is already done in bool_field_put(), so it is redundant here.  I think it can be safely removed.

It is, you're right.  I'm going to rerun the tests because this is an unrelated change.

> src/hotspot/share/prims/jvm.cpp line 612:
> 
>> 610:   // as implemented in the classic virtual machine; return 0 if object is null
>> 611:   return handle == nullptr ? 0 :
>> 612:          checked_cast<jint>(ObjectSynchronizer::FastHashCode (THREAD, JNIHandles::resolve_non_null(handle)));
> 
> What about making FastHashCode return jint?

I briefly had a look at that but that's a bit more fan-out from code in synchronizer.cpp and other callers.

> src/hotspot/share/prims/methodHandles.cpp line 910:
> 
>> 908:       InstanceKlass* defc = InstanceKlass::cast(java_lang_Class::as_Klass(clazz));
>> 909:       DEBUG_ONLY(clazz = nullptr);  // safety
>> 910:       intptr_t vmindex  = java_lang_invoke_MemberName::vmindex(mname());
> 
> Why not do the checked_cast<int> here instead of below?
> Ideally, the vmindex field would be changed to int (field offsets, itable/vtable indexes are all int), but that's more work.

Why not?  It's the same and casts the int where it's needed to be an int.  It would be a bigger change to make this an int.  Not that big because it's an injected field but too much extra for this change.

I had a look at this and it affects platform code non-trivially.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15160#discussion_r1284901764
PR Review Comment: https://git.openjdk.org/jdk/pull/15160#discussion_r1284903411
PR Review Comment: https://git.openjdk.org/jdk/pull/15160#discussion_r1284899851
PR Review Comment: https://git.openjdk.org/jdk/pull/15160#discussion_r1284905530