8364317: Explicitly document some assumptions of StringUTF16

[liach]

In #24773, people were concerned that the layout of a UTF16 byte array and a char array may be incompatible. In fact, they are - they are asserted in a corner in LibraryCallKit::inline_string_char_access in library_call.cpp.

In addition, another frequent error I see is that contributors have confused the meaning of indices in StringUTF16 - the indices are always in char array indices. I think we should make these explicit to help future maintenance.

---

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-8364317: Explicitly document some assumptions of StringUTF16 (Enhancement - P5)

Reviewers

• Raffaello Giulietti (@rgiulietti - Reviewer)
• Roger Riggs (@RogerRiggs - Reviewer)
• Volkan Yazici (@vy - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26541

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back liach! 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]

@liach This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8364317: Explicitly document some assumptions of StringUTF16

Reviewed-by: rgiulietti, rriggs, vyazici

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 44 new commits pushed to the master branch:

• c4fbfa2: 8363949: Incorrect jtreg header in MonitorWithDeadObjectTest.java
• 1b9efaa: 8364183: Shenandoah: Improve commit/uncommit handling
• b2b56cf: 8359395: XML signature generation does not support user provided SecureRandom
• ... and 41 more: https://git.openjdk.org/jdk/compare/ea0b49c36db7dce508aec7e72e73c7274d65bc15...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@liach The following label will be automatically applied to this pull request:

• core-libs

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

[mlbridge]

Webrevs

• 01: Full - Incremental (1b052e9e)
• 00: Full (933557ef)

[rgiulietti]

A good initiative, thanks @liach.
Maybe some words about endianness would be useful as well.

[RogerRiggs]

looks good

[liach]

@rgiulietti: I have added a paragraph describing the layout constraints of UTF16 byte arrays. Please check it out.

[rgiulietti]

Looks fine

[vy]

All indices and sizes for byte arrays carrying UTF-16 data are in number of
chars instead of number of bytes.

@liach, given the relatively big API surface of j.l.StringUTF16, are we certain about this?

[liach]

@liach, given the relatively big API surface of j.l.StringUTF16, are we certain about this?

I am. In fact, your update to use char made it less correct - there are a few APIs that take LATIN1 byte arrays, for which number of chars is equivalent to number of bytes.

[vy]

@liach, given the relatively big API surface of j.l.StringUTF16, are we certain about this?

I am. In fact, your update to use char made it less correct - there are a few APIs that take LATIN1 byte arrays, for which number of chars is equivalent to number of bytes.

Thanks for taking care of these details Chen, much appreciated. Changes LGTM.

[liach]

Thanks for the reviews! I hope a random future commit might pick up that extra space as a gratituity.

/integrate

[openjdk]

Going to push as commit fe09e93.
Since your change was applied there have been 45 commits pushed to the master branch:

• d194423: 8358880: Performance of parsing with DecimalFormat can be improved
• c4fbfa2: 8363949: Incorrect jtreg header in MonitorWithDeadObjectTest.java
• 1b9efaa: 8364183: Shenandoah: Improve commit/uncommit handling
• ... and 42 more: https://git.openjdk.org/jdk/compare/ea0b49c36db7dce508aec7e72e73c7274d65bc15...master

Your commit was automatically rebased without conflicts.

[openjdk]

@liach Pushed as commit fe09e93.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.