RFR: 8314265: Fix -Wconversion warnings in miscellaneous runtime code [v6]
David Holmes
dholmes at openjdk.org
Fri Aug 18 00:35:32 UTC 2023
On Thu, 17 Aug 2023 12:10:51 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> src/hotspot/share/logging/logOutput.cpp line 69:
>>
>>> 67:
>>> 68: static int tag_cmp(const LogTagType *a, const LogTagType *b) {
>>> 69: return primitive_compare(a, b);
>>
>> This looks very odd given we are dealing with pointers not primitives.
>
> Was it odd before or odd now? What we want to do is compare pointers for a sort function. This primitive_compare has been used in other places as an improvement.
It is the name `primitive_compare` - I only previously saw it used for integer types. Using it with pointers seems "wrong". Don't we have to convert to `intptr_t` to compare pointers numerically anyway?
>> src/hotspot/share/services/threadService.hpp line 107:
>>
>>> 105: static jlong get_peak_thread_count() { return _peak_threads_count->get_value(); }
>>> 106: static int get_live_thread_count() { return _atomic_threads_count; }
>>> 107: static int get_daemon_thread_count() { return _atomic_daemon_threads_count; }
>>
>> Given all the other jlong usage in these functions, and that these are used for the return value of `jlong get_long_attribute(jmmLongAttribute att)` in management.cpp, I think these should stay as jlong returning functions.
>
> If they stay jlong returns (note that these fields are in fact int), then we need to add casting to all the callers. Casting is worse than returning the correct types. If someone wants to make these fields jlong someday then they can propagate the change to the callers. This change corrects the types.
I don't follow. The fields are int so cast them to jlong before returning them. All the callers of these methods expect jlong so there can't be any issue there.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1297870904
PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1297871632
More information about the build-dev
mailing list