RFR: 8240110: Improve NULL

[Leo Korinth]

Hi!

On 06/04/2020 01:37, David Holmes wrote:

>>
>> I agree that it could be compared against NULL_WORD, do you want me to 
>> change it? (I have no opinion on this issue)
> 
> If we're trying to clean up the use of 0, NULL, NULL_WORD then I think 
> it should be changed.

Fixed in several locations.

>>>
>>>  > new NULL value does unfortunately not convert to method pointers
>>>
>>> Is that a temporary limitation? I'm not sure what progress were are 
>>> making if some pointers can use NULL and others must use 0.
>>
>> I believe this to be a temporary limitation, my understanding is that 
>> when we get c++11 nullptr will convert to method pointers as well, I 
>> could and think I should revert these changes as I did revert my 
>> implementation of NULL, would you like that? My null_t type could, to 
>> my knowledge, not auto convert to method pointers at least not without 
>> template varargs stuff that comes in later versions of the standard.
> 
> This would look better to me as NULL.

I have reverted this and a few other places.


>>
>> This is the same issue as before with string literals and macros, my 
>> arguing is that I will not try to prove that value is (or in the 
>> future always will be) a string literal, instead I will make the code 
>> work for both string literals and pointers. I hope you do not mind the 
>> extra parentheses I added around "value".
> 
> Normal style rules for macros say to use the macro arg in parentheses - 
> but there are three other uses in the same macro.
> 

I will keep the parenthesis on the line I changed and do no additional 
such change.


> 
>> I could change CHECK_0 to use NULL_WORD though and if I do, I get 
>> compiler errors. CHECK_0 is used for both pointers and integral zeroes 
>> and types that implicitly converts from either. What the original 
>> thought was is unclear to me; is CHECK_0 to be used to return things 
>> convertible from pointers? If so, what is CHECK_NULL for?
> 
> No CHECK_0 is intended for int-returning functions (which would also 
> extend to intptr_t).

It might be intended for it, but it is not used in such a way (therefore 
my remark on compile failures when using NULL_WORD)

I will do no change here.


>>
>>> ---
>>>
>>> src/hotspot/share/utilities/globalDefinitions_solstudio.hpp
>>>
>>> It isn't clear that the issue of passing 0 as NULL in a varargs 
>>> situation is now fixed in Sol Studio.
>>>
>>
>> I guess tier1-3 would fail if not? Maybe that is a false assumption of 
>> mine?
> 
> I would consider that a necessary but not sufficient condition.
> 

I reverted my NULL/NULL_WORD redefinition so it is no issue any more at 
least.

>>>
>>> src/hotspot/share/utilities/linkedlist.hpp
>>>
>>> -    return equal<E>(_data, t, NULL);
>>> +    return equal<E>(_data, t, 0);
>>>
>>> Again concerned about the method/function pointer situation.
>>
>> I am not concerned :-) but I will revert this as it is not necessary 
>> now or in the future as I reverted my original implementation and 
>> c++11 will fix this.
> 
> Ok.

Fixed

In additions to these changes I have also /reverted/ all changes to 
NULL/NULL_WORD definitions. In the end Kim did not like them, and they 
can be fixed later when we get a more modern C++. To reflect this I will 
also change the CR title to "Improve NULL usage"

Webrev:
http://cr.openjdk.java.net/~lkorinth/8240110/part1 (NULL)
http://cr.openjdk.java.net/~lkorinth/8240110/part2 (NULL_WORD)
http://cr.openjdk.java.net/~lkorinth/8240110/part3 (reverse somewhat)

new in this mail:
http://cr.openjdk.java.net/~lkorinth/8240110/part4 (improvements 
suggested by David)
http://cr.openjdk.java.net/~lkorinth/8240110/part5 (reverse my changes 
to NULL/NULL_WORD itself)

total combined change:
http://cr.openjdk.java.net/~lkorinth/8240110/1

Testing:
running tier 1-3 (looks good so far)

Thanks,
Leo