8346035: [lworld] javadoc for value objects when enable preview #1316
Conversation
Added text and decoration to highlight preview behavior of identity classes that are value classes with --enable-preview
|
👋 Welcome back rriggs! A progress list of the required criteria for merging this PR into |
|
@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: 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 1 new commit pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
| * The {@code equals} method should be used for comparisons. | ||
| * class; programmers should treat instances that are {@linkplain #equals(Object) equal} | ||
| * as interchangeable and should not use instances for synchronization, mutexes, or | ||
| * with {@linkplain java.lang.ref.Reference object references}. |
There was a problem hiding this comment.
Should we add the preview warning with rewords, like
When preview features are enabled, all implementations of {@code ZoneId} are {@linkplain Class#isValue value class}. ...
There was a problem hiding this comment.
There's more to do before ZoneId can be a Valhalla value class.
Its subtype ZoneOffset has a non-final field that caches ZoneRules. So ZoneOffset can't be a value class.
RogerRiggs
changed the title
| * <div class="preview-comment"> | ||
| * When preview features are enabled, {@code Record} is | ||
| * an abstract {@linkplain Class#isValue value class}. | ||
| * Subclasses of {@code Number} can be either an {@linkplain Class#isIdentity identity class} |
| visible change in the behavior of the class's methods;</li> | ||
| <li>the class performs no synchronization using an instance's monitor;</li> | ||
| <li>the class does not declare (or has deprecated any) accessible constructors;</li> |
There was a problem hiding this comment.
This one is still relevant, as a concrete case of the next bullet, right? A value-based class should not let clients control identity creation.
There was a problem hiding this comment.
Could maybe soften to "(or discourages use of any)"...
| serialization, or any other identity-sensitive mechanism.</p> | ||
|
|
||
| <p>Synchronization on instances of value-based classes is strongly discouraged, | ||
| because the programmer cannot guarantee exclusive ownership of the | ||
| associated monitor.</p> | ||
|
|
||
| <p>Identity-related behavior of value-based classes may change in a future release. | ||
| For example, synchronization may fail.</p> | ||
| <p>When preview features are enabled, use of value class instances for synchronization, mutexes, or with |
There was a problem hiding this comment.
A value-based class is not necessarily a value class under preview.
I think this "behavior may change" disclaimer is trying to cover two cases, which should probably be identified directly:
- The class may choose to allocate/cache instances differently
- The class may choose to become a value class, and then [things may fail]
| * occur. For example, in a future release, synchronization may fail. | ||
| * class; programmers should treat instances that are {@linkplain #equals(Object) equal} | ||
| * as interchangeable and should not use instances for synchronization, mutexes, or | ||
| * with {@linkplain java.lang.ref.Reference object references}. |
There was a problem hiding this comment.
We have a preview feature disclaimer for References.
Is there a similar one for mutexes? (What exactly do you mean by "mutexes"? Is it distinct from "synchronization"?)
Updated javadoc for value objects when preview is enabled.
Added text and decoration to highlight preview behavior of identity classes that are value classes with --enable-preview
The decorations are similar to the existing preview highlights for Preview classes and methods.
The updates include all of the wrapper classes, java.time, and java.util.Optional* and other classed annotated with @valuebased.
The ValueBased is updated to describe the behavior when --enable-preview.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/1316/head:pull/1316$ git checkout pull/1316Update a local copy of the PR:
$ git checkout pull/1316$ git pull https://git.openjdk.org/valhalla.git pull/1316/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1316View PR using the GUI difftool:
$ git pr show -t 1316Using diff file
Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/1316.diff
Using Webrev
Link to Webrev Comment