Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8257212: (bf spec) Clarify byte order of the buffer returned by CharBuffer.subsequence(int,int) #2049

Closed
wants to merge 2 commits into from

Conversation

@c-cleary
Copy link
Contributor

@c-cleary c-cleary commented Jan 12, 2021

Specification for CharBuffer::subSequence was missing clarification on the byte order of the returned CharBuffer. The returned order will always be the same as that of the original buffer. For example, subSequence() called on a CharBuffer with Little-Endian Byte Order will return a CharBuffer of Little-Endian Order. The specification has been changed to reflect this.

In addition to this, some additional testing was added to improve test coverage of this behavior. Testing for the Byte Order of different types of Buffer is generated via the template Order-X.java.template which serves to verify the original Byte Order of a Buffer and subsequently verify that the same Byte Order is present after operations such as CharBuffer::subSequence

CSR: https://bugs.openjdk.java.net/browse/JDK-8259638


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8257212: (bf spec) Clarify byte order of the buffer returned by CharBuffer.subsequence(int,int)

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2049/head:pull/2049
$ git checkout pull/2049

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Jan 12, 2021

👋 Welcome back ccleary! 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
Copy link

@openjdk openjdk bot commented Jan 12, 2021

@c-cleary The following label will be automatically applied to this pull request:

  • nio

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.

@openjdk openjdk bot added the nio label Jan 12, 2021
@ChrisHegarty
Copy link
Member

@ChrisHegarty ChrisHegarty commented Jan 12, 2021

/csr

@openjdk
Copy link

@openjdk openjdk bot commented Jan 12, 2021

@c-cleary 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:

8257212: (bf spec) Clarify byte order of the buffer returned by CharBuffer.subsequence(int,int)

Reviewed-by: chegar, bpb, alanb

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 74 new commits pushed to the master branch:

  • b01a15e: 8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test
  • 6d4a593: 8259627: Potential memory leaks in JVMTI after JDK-8227745
  • 2c8e337: 8259622: TreeMap.computeIfAbsent deviates from spec
  • d701bab: Merge
  • 4307fa6: 8253505: JFR: onFlush invoked out of order with a sorted event stream
  • 0148adf: 8255120: C2: assert(outer->outcnt() >= phis + 2 && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
  • 90960c5: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed
  • e3b548a: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens
  • 978bed6: 8259522: Apply java.io.Serial annotations in java.desktop
  • bf28f92: 8259713: Fix comments about ResetNoHandleMark in deoptimization
  • ... and 64 more: https://git.openjdk.java.net/jdk/compare/4697cfa4b01792c04becba3632833559e4b755b7...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@ChrisHegarty, @bplb, @AlanBateman) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk
Copy link

@openjdk openjdk bot commented Jan 12, 2021

@ChrisHegarty has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@c-cleary please create a CSR request and add link to it in JDK-8257212. This pull request cannot be integrated until the CSR request is approved.

@openjdk openjdk bot removed the ready label Jan 12, 2021
@c-cleary c-cleary marked this pull request as ready for review Jan 12, 2021
@openjdk openjdk bot added the rfr label Jan 12, 2021
@c-cleary
Copy link
Contributor Author

@c-cleary c-cleary commented Jan 12, 2021

@mlbridge
Copy link

@mlbridge mlbridge bot commented Jan 12, 2021

Webrevs

@bplb
Copy link
Member

@bplb bplb commented Jan 12, 2021

In X-Buffer.java.template, s/2020/2021/ in the copyright year; in Order-X.java.template add 2021 as second copyright year.

@openjdk openjdk bot added ready and removed csr labels Jan 12, 2021
@c-cleary
Copy link
Contributor Author

@c-cleary c-cleary commented Jan 13, 2021

In X-Buffer.java.template, s/2020/2021/ in the copyright year; in Order-X.java.template add 2021 as second copyright year.

2021 is now added to all copyright notices... classic start of the year mistake!

bplb
bplb approved these changes Jan 13, 2021
@c-cleary
Copy link
Contributor Author

@c-cleary c-cleary commented Jan 14, 2021

/integrate

@openjdk openjdk bot added the sponsor label Jan 14, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jan 14, 2021

@c-cleary
Your change (at version 4f90a20) is now ready to be sponsored by a Committer.

@ChrisHegarty
Copy link
Member

@ChrisHegarty ChrisHegarty commented Jan 15, 2021

/sponsor

@openjdk openjdk bot closed this Jan 15, 2021
@openjdk openjdk bot added integrated and removed sponsor labels Jan 15, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Jan 15, 2021

@ChrisHegarty @c-cleary Since your change was applied there have been 75 commits pushed to the master branch:

  • 0ec2c96: 8259820: JShell does not handle -source 8 properly
  • b01a15e: 8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test
  • 6d4a593: 8259627: Potential memory leaks in JVMTI after JDK-8227745
  • 2c8e337: 8259622: TreeMap.computeIfAbsent deviates from spec
  • d701bab: Merge
  • 4307fa6: 8253505: JFR: onFlush invoked out of order with a sorted event stream
  • 0148adf: 8255120: C2: assert(outer->outcnt() >= phis + 2 && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
  • 90960c5: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed
  • e3b548a: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens
  • 978bed6: 8259522: Apply java.io.Serial annotations in java.desktop
  • ... and 65 more: https://git.openjdk.java.net/jdk/compare/4697cfa4b01792c04becba3632833559e4b755b7...master

Your commit was automatically rebased without conflicts.

Pushed as commit 707bce0.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants