8326716: JVMTI spec: clarify what nullptr means for C/C++ developers

[sspitsyn]

The following RFE was fixed recently:
8324680: Replace NULL with nullptr in JVMTI generated code

It replaced all the NULL's in the generated spec withnullptr. JVMTI agents can be developed in C or C++.
This update is to make it clear that nullptr is C programming language null pointer.

I think we do not need a CSR for this fix.

Testing: N/A (not needed)

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8326716: JVMTI spec: clarify what nullptr means for C/C++ developers (Enhancement - P4)

Reviewers

• Kim Barrett (@kimbarrett - Reviewer) ⚠️ Review applies to 9fe639e1

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19257/head:pull/19257
$ git checkout pull/19257

Update a local copy of the PR:
$ git checkout pull/19257
$ git pull https://git.openjdk.org/jdk.git pull/19257/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19257

View PR using the GUI difftool:
$ git pr show -t 19257

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19257.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sspitsyn! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@sspitsyn This change is no longer ready for integration - check the PR body for details.

[openjdk]

@sspitsyn The following labels will be automatically applied to this pull request:

• hotspot
• serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 04: Full - Incremental (ed2eff27)
• 03: Full - Incremental (48ba8f5d)
• 02: Full - Incremental (4e1c48a1)
• 01: Full - Incremental (9fe639e1)
• 00: Full (fd0e8d43)

[kimbarrett]

Looks good.

[dholmes-ora]

But this clarification doesn't actually clarify that the rest of the spec uses nullptr. Based on the proposed wording I would expect things like:

The function may return <code>nullptr</code>

to say

The function may return a null pointer

[merykitty]

This may be a little unnecessary rigor, but I believe that nullptr is not a null pointer. nullptr is the pointer literal that can be implicitly converted to a null pointer value of any pointer type and any pointer to member type. And I think the thing returned here is a null pointer, not nullptr.

[sspitsyn]

I'm not sure I understand this comment. Sorry.

[kimbarrett]

But this clarification doesn't actually clarify that the rest of the spec uses nullptr. Based on the proposed wording I would expect things like:

The function may return <code>nullptr</code>

to say

The function may return a null pointer

Looking at this again, I think I agree with @dholmes-ora .

Some of the relevant places are text, and should be using "null pointer".
Some are example code or the like. Those should be using NULL rather than
nullptr, since we have this text early on:

"Unless otherwise stated, all examples and declarations in this
specification use the C language."

I didn't find any that were described as C++ rather than C.

So JDK-8324680 was somewhat mistaken about what needed to be done, and what
was done.

[sspitsyn]

So JDK-8324680 was somewhat mistaken about what needed to be done, and what
was done.

The jvmti.xml is used to generate several things with the XSL scripts:

• JVMTI spec (jvmti.html)
• JVMTI api (jvmti.h)
• jvmtiEnter.cpp, jvmtiEnterTrace.cpp

In fact, it is pretty tricky to separate these usage aspects of nullptr or NULL.
One of the approaches is to undo the JDK-8324680.
Please, let me know if you prefer this path.

[openjdk]

@sspitsyn this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout b33
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[sspitsyn]

But this clarification doesn't actually clarify that the rest of the spec uses nullptr. Based on the proposed wording I would expect things like:

The function may return nullptr

to say

The function may return a null pointer

Okay. I've made a fix to replace in the docs nullptr with null pointer as you suggested.

[dholmes-ora]

Okay. I've made a fix to replace in the docs nullptr with null pointer as you suggested.

What I suggested was

returns a null pointer

in place of

returns nulllptr

but "a null pointer" doesn't always look right either e.g. "was a null pointer" would be better as just "was null".

I think using non-code-font "null" to represent the concept of null-ness would be fine: "returns null", "is null", "was null"

[sspitsyn]

Thanks, David. I've done one more attempt to correct it. Please, let me know if it is still wrong.

[dholmes-ora]

The general rules are to either say "a null pointer" (possibly with capital A depending on context), or just "null". And in most cases you could choose either. I made various suggestions but really it is up to you. It is hard to get a sense of consistency from these small fragments.

The word "null" should never be in code font as it is not a programming language entity.

Thanks for your patience and perseverance on this.

[sspitsyn]

The general rules are to either say "a null pointer" (possibly with capital A depending on context), or just "null". And in most cases you could choose either. I made various suggestions but really it is up to you. It is hard to get a sense of consistency from these small fragments.

The word "null" should never be in code font as it is not a programming language entity.

Thank you, David. This is really useful.
I've pushed an update with all changes you suggested.

[plummercj]

Why describe what is meant by a "null pointer" here when it is not done elsewhere?

[dholmes-ora]

The intent is to provide a definition of what a null pointer is, for both C and C++ programs. Is there a better place to do that so that elsewhere the spec can simply to refer to "a null pointer" or "null"?

[sspitsyn]

Thanks, David. I also feel this clarification is still useful.

[AlanBateman]

I think this is the right place but it is only for return values. There are a few functions where a parameter value can be a null pointer, e.g. in GetThreadState, SuspendThread, GetOwnedMonitorInfo the thread parameter can be a null pointer to mean the current thread. I don't think the introduction section has anywhere right now to reference for parameters that can be NULL/nullptr.

[plummercj]

Yes, my point was that this section is only for return values. The section is titled "Function Return Values". Maybe we should add another short section just before this one to describe what is meant by "null pointer".

[sspitsyn]

Okay, thanks. What about the following: :

diff --git a/src/hotspot/share/prims/jvmti.xml b/src/hotspot/share/prims/jvmti.xml
index a6ebd0d42c5..a81014c70bb 100644
--- a/src/hotspot/share/prims/jvmti.xml
+++ b/src/hotspot/share/prims/jvmti.xml
@@ -995,7 +995,10 @@ jvmtiEnv *jvmti;
     across threads and are created dynamically.
   </intro>
 
-  <intro id="functionReturn" label="Function Return Values">
+  <intro id="functionReturn" label="Function Parameters and Return Values">
+    There are a few <jvmti/> functions where a parameter value can be a null pointer
+    (C <code>NULL</code> or C++ <code>nullptr</code>), e.g. the thread parameter
+    can be a null pointer to mean the current thread.
     <jvmti/> functions always return an
     <internallink id="ErrorSection">error code</internallink> via the
     <datalink id="jvmtiError"/> function return value.
@@ -1004,7 +1007,7 @@ jvmtiEnv *jvmti;
     In some cases, <jvmti/> functions allocate memory that your program must
     explicitly deallocate. This is indicated in the individual <jvmti/>
     function descriptions.  Empty lists, arrays, sequences, etc are
-    returned as a null pointer (C <code>NULL</code> or C++ <code>nullptr</code>).
+    returned as a null pointer.
     <p/>
     In the event that the <jvmti/> function encounters
     an error (any return value other than <code>JVMTI_ERROR_NONE</code>) the values

I can try to add a couple of more examples where a null pointer can be passed as a parameter value if it is desirable.