<AWT Dev> [9] Review request: new macro for conversion to jboolean

[roger riggs]

Hi,

For now I would just use the explicit ternary operator.
The macro would have a very narrow use.
Improving the ability to do static checking on JNI types would be a 
larger undertaking.

Roger



On 3/21/2014 5:38 PM, Mike Duigou wrote:
> What if we just abandon adding an IS_TRUE / TO_JBOOLEAN macro for now.
>
> I would like to address adding (jboolean) casts to jni.h which I suspect would help with the static analysis issues encountered.
>
> For now just use the explicit ? JNI_TRUE : JNI_FALSE whenever a jboolean value is needed and we can refactor the naming of the macro, if any is required, later.
>
> Mike
>
> On Mar 21 2014, at 08:10 , roger riggs <roger.riggs at oracle.com> wrote:
>
>> Hi Sergey,
>>
>> I didn't see any examples of the use case in this thread.  The proposed name seems bulky and clumsy.
>>
>> In the snippet provided, it did not seem necessary to force the cast to jboolean.
>> Both JNI_TRUE and JNI_FALSE are untyped constants.
>>
>> The macro would just as useful (if I understand the cases) without the cast.
>>
>> How useful is a simple definition as:
>>
>> #define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
>>
>> then it would look ok to see these in sources:
>>
>>       return IS_TRUE(obj);
>>
>>       if (IS_TRUE(obj)) {....}
>>
>>       jboolean ret = IS_TRUE(obj);
>>
>> The general purpose usage matches the general C conventions for
>> true and false and match the JNI semantics.
>>
>> Roger
>>
>>
>>
>> On 3/19/2014 7:43 PM, Sergey Bylokhov wrote:
>>> Thanks.
>>> So if nobody objects, the final version will be:
>>>
>>> #define IS_NULL(obj) ((obj) == NULL)
>>> #define JNU_IsNull(env,obj) ((obj) == NULL)
>>> +#define TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
>>>
>>>
>>> On 3/20/14 12:07 AM, roger riggs wrote:
>>>> Hi,
>>>>
>>>> Well...  When JNU_RETURN was added, there was a long discussion about
>>>> NOT using JNU unless the JNI environment was an argument to the macro.
>>>>
>>>> So, the simple name is preferred.
>>>>
>>>> Roger
>>>>
>>>> On 3/19/2014 4:08 PM, Phil Race wrote:
>>>>> PS .. so maybe whilst you are touching this file you could do
>>>>> #define JNU_CHECK_NULL CHECK_NULL
>>>>> #define JNU_CHECK_NULL_RETURN CHECK_NULL_RETURN
>>>>>
>>>>> so we could migrate to these (clearer) ones
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 3/19/2014 1:05 PM, Phil Race wrote:
>>>>>> I think having it start with "JNU_" is a good suggestion.
>>>>>> I've been wondering over the last week or so if it would not have been
>>>>>> better to have CHECK_NULL called JNU_CHECK_NULL to reduce collision chances
>>>>>> and to make it clearer where it comes from ..
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>> On 3/19/2014 12:49 PM, Mike Duigou wrote:
>>>>>>> Definitely a useful macro.
>>>>>>>
>>>>>>> I too would prefer a name like TO_JBOOLEAN since it reveals the result type. Also all uppercase to identify it as a macro. If we are paranoid and want to reduce the chance of a name collision then JNU_TO_JBOOLEAN perhaps.
>>>>>>>
>>>>>>> I would also define the macro as:
>>>>>>>
>>>>>>> #define JNU_TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)
>>>>>>>
>>>>>>> so that the type of the result is explicit. Unfortunately jni.h doesn't define JNI_TRUE or false with a cast to jboolean as they probably should.
>>>>>>>
>>>>>>> Mike
>>>>>>>
>>>>>>> On Mar 19 2014, at 11:36 , Sergey Bylokhov <sergey.bylokhov at oracle.com> wrote:
>>>>>>>
>>>>>>>> Thanks Anthony!
>>>>>>>>
>>>>>>>> Can somebody from the core-libs team take a look?
>>>>>>>>
>>>>>>>> On 3/17/14 10:31 PM, Anthony Petrov wrote:
>>>>>>>>> Personally, I'd call it to_jboolean(obj), but IS_TRUE(obj) sounds good to me too. Either way, I'm fine with the fix.
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> best regards,
>>>>>>>>> Anthony
>>>>>>>>>
>>>>>>>>> On 3/17/2014 7:01 PM, Sergey Bylokhov wrote:
>>>>>>>>>> Hello.
>>>>>>>>>> This review request is for the new macro, which simplify conversion to
>>>>>>>>>> jboolean. It will be useful for fixing parfait warnings.
>>>>>>>>>>
>>>>>>>>>> We have a lot of places, where we cast some type to jboolean:
>>>>>>>>>>
>>>>>>>>>> BOOL = retVal;
>>>>>>>>>> return (jboolean) retVal;
>>>>>>>>>>
>>>>>>>>>> WARNING: Expecting value of JNI primitive type jboolean: mismatched
>>>>>>>>>> value retVal with size 32 bits, retVal used for conversion to int8 in return
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> +++ b/src/share/native/common/jni_util.h    Mon Mar 17 18:28:48 2014 +0400
>>>>>>>>>> @@ -277,6 +277,7 @@
>>>>>>>>>>
>>>>>>>>>>   #define IS_NULL(obj) ((obj) == NULL)
>>>>>>>>>>   #define JNU_IsNull(env,obj) ((obj) == NULL)
>>>>>>>>>> +#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)
>>>>>>>>>>
>>>>>>>>>> I am not sure about the name, probably someone have a better suggestion?
>>>>>>>>>>
>>>>>>>>>> The fix is for jdk9/dev.
>>>>>>>>>>
>>>>>>>>>> -- 
>>>>>>>>>> Best regards, Sergey.
>>>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>