RFR 8208399: Metadata methods print_(value_)on_maybe_null() compare 'this' to NULL

Ioi Lam ioi.lam at oracle.com
Tue Jul 31 01:00:22 UTC 2018


There are 92 cases of '[(]NULL ==' vs 3403 cases of '== NULL[)]' in the 
hotspot source code.

It's been claimed that (NULL == variable) is superior because if you 
type "=" by mistake, or delete one of the equal signs by mistake, the 
compiler will catch you.

- Ioi


On 7/30/18 2:52 PM, David Holmes wrote:
> On 31/07/2018 4:16 AM, Harold David Seigel wrote:
>> Thanks Gerard!
>>
>> I'll make that change before pushing the fix.
>
> Hmmm is that part of our coding guidelines? It is not something we 
> generally follow. Personally I've never liked the "if (<constant> == 
> variable)" form as it just reads wrong to me.
>
> David
>
>> Harold
>>
>>
>> On 7/30/2018 2:05 PM, Gerard Ziemski wrote:
>>> Looks good.
>>>
>>> In src/hotspot/share/oops/metadata.hpp tiny nitpick - I was once 
>>> told that instead of:
>>>
>>> +    if (m == NULL)
>>>
>>> we should be using:
>>>
>>> +    if (NULL == m)
>>>
>>> but I guess the entire file would have to use that syntax.
>>>
>>>
>>>
>>> cheers
>>>
>>>> On Jul 27, 2018, at 12:15 PM, Harold David Seigel 
>>>> <harold.seigel at oracle.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Please review this JDK-12 fix for bug JDK-8208399.  The fix changes 
>>>> function print_value_on_maybe_null() into a static function to 
>>>> avoid comparisons between 'this' and NULL.  It also deletes unused 
>>>> methods print_maybe_null() and print_on_maybe_null().
>>>>
>>>> Open Webrev: 
>>>> http://cr.openjdk.java.net/~hseigel/bug_8208399/webrev/index.html
>>>>
>>>> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8208399
>>>>
>>>> This fix was regression tested by running Mach5 tiers 1 and 2 tests 
>>>> and builds on Linux-X64, Windows, Solaris Sparc, and Mac OS X, 
>>>> running tiers 3-5 tests on Linux-x64, and by running JCK-11 Lang 
>>>> and VM tests on Linux-x64.
>>>>
>>>> Thanks, Harold
>>>>
>>



More information about the hotspot-runtime-dev mailing list