valhalla
valhalla copied to clipboard
openjdk
8346307: [lworld] Clarify identity vs value in Class, Objects, and document limitations of value objects
Add APINote and javadoc for IdentityException where it will be useful to know that identity or value objects are treated differently. Simplified WeakHashMap javadoc updates for IdentityException. Added note to System.identityHashCode to include value objects. Added to class javadoc for IdentityHashMap for value objects.
Progress
- [x] Change must not contain extraneous whitespace
Issue
- JDK-8346307: [lworld] Clarify identity vs value in Class, Objects, and document limitations of value objects (Bug - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1327/head:pull/1327
$ git checkout pull/1327
Update a local copy of the PR:
$ git checkout pull/1327
$ git pull https://git.openjdk.org/valhalla.git pull/1327/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1327
View PR using the GUI difftool:
$ git pr show -t 1327
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1327.diff
Using Webrev
:wave: Welcome back rriggs! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@RogerRiggs 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:
8346307: [lworld] Clarify identity vs value in Class, Objects, and document limitations of value objects
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 2 new commits pushed to the lworld branch:
- a565d8bee48e4cf6bbcbb7e360c94a05fed02e3f: 8359345: [lworld] C2 crashes with -XX:ForceNonTearable=*
- 051283b610d1b49446bb3838e7c4c9d87c8616cd: 8360530: [lworld] CDS does not preload classes listed in the LoadableDescriptors attribute
Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld branch.
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 lworld branch, type /integrate in a new comment.
Webrevs
- 10: Full - Incremental (b044e43e)
- 09: Full - Incremental (023f4f81)
- 08: Full (6cbade17)
- 07: Full (b6cd55b5)
- 06: Full (3d774c91)
- 05: Full (2cd87766)
- 04: Full (b26dbfaa)
- 03: Full - Incremental (d450add4)
- 02: Full - Incremental (850923bd)
- 01: Full - Incremental (2aae8e67)
- 00: Full (339bc421)
⚠️ @RogerRiggs This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).
@RogerRiggs This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Does this intend to wait for openjdk/jdk#23395 or will it migrate from HTML after the taglet is available?
I've been trying out the various incarnations of the javadoc support to see how they work, but that can be a separate PR when it settles.
@RogerRiggs This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@RogerRiggs This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.
@RogerRiggs This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
PreviewNote isn't merged yet; we can revisit this one once that taglet is available here.
The PreviewNote tag has been delayed, the plan is to integrate these changes until it settles down.
This is still incorrect:
Unless Preview is enabled, there is no such thing as a value class (according to the spec). Until JEP 401 is enabled, all classes are identity classes. So it would be incorrect to report that primitive classes, interfaces, and void.class are value classes.
We should document explicitly that all classes, including primitives and void, are considered identity when preview features are not enabled.
We should document explicitly that all classes, including primitives and void, are considered identity when preview features are not enabled.
Primitives and void are not classes and never have identity.
The primitive wrappers and Void are classes have identity if that's what you're meaning.
Interface classes never have identity (do not have ACC_SUPER/ACC_IDENTITY).
Interface classes with --enable-preview are reported as value classes.
Going to push as commit 6541b8be1d4c44245cdd62f85a7d7e943b084140.
Since your change was applied there have been 4 commits pushed to the lworld branch:
- ed13d1fab2f96f46c203fb0c20f54333c1c38b84: 8329234: [lworld] compiler/gcbarriers/TestZGCBarrierElision.java fails after JDK-8329205
- 01af2b48708ca67e8bd9bea74b78bed6e2c2a180: 8336003: [lworld] TestLWorld::test151 triggers "Should have been buffered" assert
- a565d8bee48e4cf6bbcbb7e360c94a05fed02e3f: 8359345: [lworld] C2 crashes with -XX:ForceNonTearable=*
- ... and 1 more: https://git.openjdk.org/valhalla/compare/81fbc4147fe1184dc870208b8d73b6c057861119...lworld
Your commit was automatically rebased without conflicts.
@RogerRiggs Pushed as commit 6541b8be1d4c44245cdd62f85a7d7e943b084140.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.