On Fri, 18 Aug 2023 00:24:07 GMT, David Holmes <dhol...@openjdk.org> wrote:

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

You don't need to cast from int to jlong.   The callers of these methods expect 
int:


src/hotspot/share/services/threadService.cpp:1103:55: warning: conversion from 
'jlong' {aka 'long int'} to 'int' may change value [-Wconversion]
 1103 |   int init_size = ThreadService::get_live_thread_count();
      |                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~


Changing int to jlong here, gets a new warning:


src/hotspot/share/services/threadService.cpp:1104:54: warning: conversion from 
'jlong' {aka 'long int'} to 'int' may change value [-Wconversion]
 1104 |   _threads_array = new GrowableArray<instanceHandle>(init_size);
      |                                                      ^~~~~~~~~


If you look up in the header file, the field is an int.  It makes sense to 
return an int:


  static volatile int  _atomic_threads_count;
  static volatile int  _atomic_daemon_threads_count;


Correcting the types means returning a type that matches the declaration so 
that its consistent.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15233#discussion_r1297894123

Reply via email to