RFR: 8240110: Improve NULL

[Kim Barrett]

> On Apr 19, 2020, at 6:13 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> On 20/04/2020 3:08 am, Leo Korinth wrote:
>> On 17/04/2020 08:45, David Holmes wrote:
>>> Hi Leo,
>>> 
>>> I've taken a look at the complete set of changes and the only thing I strongly object to is the change to:
>>> 
>>> src/hotspot/share/jvmci/jvmciExceptions.hpp
>>> 
>>> as it just doesn't make any sense to me. If you were to write the expanded macros out in full you would write the code as it currently is defined in the macro. If a method returns a pointer you would write:
>>> 
>>> return NULL;
>>> 
>>> If a method returns a jint/jbyte/jlong etc you would write
>>> 
>>> return 0;
>>> 
>>> Cheers,
>>> David
>> I totally agree with you and the change needed was smaller than I rememberd, I should have fixed this directly. Returning NULL for an object that is not of pointer type seems just wrong, but returning a JVMCIObjectArray(NULL) seems okay.
>> What about this?
>> #define JVMCI_CHECK_0             JVMCI_CHECK_(NULL_WORD)
> 
> Why does this integer returning macro need to return NULL_WORD? As I said above if writing this directly I would write "return 0;" in the code. Is this just an attempt under the covers to (eventually) avoid misusing JVMCI_CHECK_0 where JVMCI_CHECK_NULL should have been used?

I agree that NULL_WORD has the wrong implication here.

As you correctly surmise, the problem with returning `0` is that it is
a null pointer constant. It would be nice if JVMCI_CHECK_0 could not
be used in a context where a pointer result is expected, and instead
require JVMCI_CHECK_NULL in such cases. That was the point of my
suggestion of using

#define JVMCI_CHECK_0  JVMCI_CHECK_(int(0))

The expression `int(0)` is not a null pointer constant, so won't
implicitly convert to a null pointer.

I realized later that there's a (currently theoretical?) problem with
this; if the result type is an unsigned integral value then this may
(someday?) produce a signed to unsigned conversion warning. (The
proposed use of NULL_WORD has the same problem.) I think there's not a
way to eliminate that possibility prior to C++11, other than by
introducing and using JVMCI_CHECK_0U to return an unsigned zero value.
But since I think that problem is currently theoretical, and can be
solved with C++11, I'm not concerned.

>> #define JVMCI_CHECK_NULL          JVMCI_CHECK_(NULL)
>> and 3 changes of JVMCI_CHECK_NULL to JVMCI_CHECK_(JVMCIObjectArray(NULL))
> 
> I'm not sure specifically where those changes are.
> 
> In general I think I prefer to see an explicit constructor passed NULL rather than some implicit conversion, but that is a stylistic matter and the implicit conversion form seems to be used quite extensively.

The question is, given class Foo, which has a conversion from pointer
constructor and is default constructable, which is preferable:

Foo foo1 = NULL;
Foo foo2(NULL);
Foo foo3;

and similarly, with functions taking a Foo:

function1(NULL)
function2(Foo(NULL)) 
function3(Foo()) 

We have occurrences of both variant #1 usages. The proposal is to
instead use the variant #2 style. My own preference would be for the
variant #3 style. Both #2 and #3 should also declare the conversion
constructor "explicit" to actively prevent variant #1; that's missing
from the current proposal.