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

8254275: [valhalla/jep390] Revise "value-based class" & apply to wrappers #222

Closed
wants to merge 11 commits into from
@@ -41,13 +41,19 @@
* {@code boolean} in an object. An object of type
* {@code Boolean} contains a single field whose type is
* {@code boolean}.
* <p>
* In addition, this class provides many methods for
*
* <p>In addition, this class provides many methods for
* converting a {@code boolean} to a {@code String} and a
* {@code String} to a {@code boolean}, as well as other
* constants and methods useful when dealing with a
* {@code boolean}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* @author Arthur van Hoff
* @since 1.0
*/
@@ -48,6 +48,12 @@
* byte}, as well as other constants and methods useful when dealing
* with a {@code byte}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* @author Nakul Saraiya
* @author Joseph D. Darcy
* @see java.lang.Number
@@ -122,6 +122,12 @@
* encoding. For more information on Unicode terminology, refer to the
* <a href="http://www.unicode.org/glossary/">Unicode Glossary</a>.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* @author Lee Boynton
* @author Guy Steele
* @author Akira Tanaka
@@ -46,6 +46,12 @@
* constants and methods useful when dealing with a
* {@code double}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* @author Lee Boynton
* @author Arthur van Hoff
* @author Joseph D. Darcy
@@ -45,6 +45,12 @@
* constants and methods useful when dealing with a
* {@code float}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* @author Lee Boynton
* @author Arthur van Hoff
* @author Joseph D. Darcy
@@ -50,6 +50,12 @@
* {@code int}, as well as other constants and methods useful when
* dealing with an {@code int}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* <p>Implementation note: The implementations of the "bit twiddling"
* methods (such as {@link #highestOneBit(int) highestOneBit} and
* {@link #numberOfTrailingZeros(int) numberOfTrailingZeros}) are
@@ -50,6 +50,12 @@
* long}, as well as other constants and methods useful when dealing
* with a {@code long}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* <p>Implementation note: The implementations of the "bit twiddling"
* methods (such as {@link #highestOneBit(long) highestOneBit} and
* {@link #numberOfTrailingZeros(long) numberOfTrailingZeros}) are
@@ -81,12 +81,11 @@
* <p>
* The {@code ProcessHandle} static factory methods return instances that are
* <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>,
* immutable and thread-safe.
* Use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on these instances of
* {@code ProcessHandle} may have unpredictable results and should be avoided.
* Use {@link #equals(Object) equals} or
* {@link #compareTo(ProcessHandle) compareTo} methods to compare ProcessHandles.
* immutable and thread-safe. Programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may occur.
* Use the {@code equals} or {@link #compareTo(ProcessHandle) compareTo} methods
* to compare ProcessHandles.
*
* @see Process
* @since 9
@@ -941,11 +941,11 @@ public static Version version() {
* $VNUM(-$PRE)?
* </pre></blockquote>
*
* <p>This is a <a href="./doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code Version} may have unpredictable results and should be avoided.
* </p>
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.</p>
*
* @since 9
*/
@@ -47,6 +47,12 @@
* {@code short}, as well as other constants and methods useful when
* dealing with a {@code short}.
*
* <p>This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; programmers should treat instances that are
* {@linkplain #equals(Object) equal} as interchangeable and should not
* use instances for synchronization, or unpredictable behavior may
* occur.
*
* @author Nakul Saraiya
* @author Joseph D. Darcy
* @see java.lang.Number
@@ -57,8 +57,8 @@
* <p>Constants describing various common constants (such as {@link ClassDesc}
* instances for platform types) can be found in {@link ConstantDescs}.
*
* <p>Implementations of {@linkplain ConstantDesc} must be
* <a href="../doc-files/ValueBased.html">value-based</a> classes.
* <p>Implementations of {@linkplain ConstantDesc} should be immutable
* and their behavior should not rely on object identity.
*
* <p>Non-platform classes should not implement {@linkplain ConstantDesc} directly.
* Instead, they should extend {@link DynamicConstantDesc} (as {@link EnumDesc}
@@ -41,8 +41,8 @@
* A <a href="package-summary.html#nominal">nominal descriptor</a> for an
* {@code invokedynamic} call site.
*
* <p>Concrete subtypes of {@linkplain DynamicCallSiteDesc} must be
* <a href="../doc-files/ValueBased.html">value-based</a>.
* <p>Concrete subtypes of {@linkplain DynamicCallSiteDesc} should be immutable
* and their behavior should not rely on object identity.
*
* @since 12
*/
@@ -49,8 +49,8 @@
* dynamic constant (one described in the constant pool with
* {@code Constant_Dynamic_info}.)
*
* <p>Concrete subtypes of {@linkplain DynamicConstantDesc} must be
* <a href="../doc-files/ValueBased.html">value-based</a>.
* <p>Concrete subtypes of {@linkplain DynamicConstantDesc} should be immutable
* and their behavior should not rely on object identity.
*
* @param <T> the type of the dynamic constant
*
@@ -31,36 +31,38 @@
<body>
<h1 id="ValueBased">{@index "Value-based Classes"}</h1>

Some classes, such as <code>java.util.Optional</code> and
<code>java.time.LocalDateTime</code>, are <em>value-based</em>. Instances of a
value-based class:
Some classes, such as <code>java.lang.Integer</code> and
<code>java.time.LocalDate</code>, are <em>value-based</em>.
A value-based class has the following properties:
<ul>
<li>are final and immutable (though may contain references to mutable
objects);</li>
<li>have implementations of <code>equals</code>,
<code>hashCode</code>, and <code>toString</code> which are computed
solely from the instance's state and not from its identity or the state
of any other object or variable;</li>
<li>make no use of identity-sensitive operations such as reference
equality (<code>==</code>) between instances, identity hash code of
instances, or synchronization on an instances's intrinsic lock;</li>
<li>are considered equal solely based on <code>equals()</code>, not
based on reference equality (<code>==</code>);</li>
This conversation was marked as resolved by dansmithcode
Comment on lines -47 to -48

This comment has been minimized.

@dansmithcode

dansmithcode Oct 9, 2020
Author Collaborator

I removed this bullet because I don't think it really means anything. We already said classes are required not to expose identity through their methods, and to override 'equals'

<li>do not have accessible constructors, but are instead instantiated
through factory methods which make no commitment as to the identity
of returned instances;</li>
<li>are <em>freely substitutable</em> when equal, meaning that interchanging
any two instances <code>x</code> and <code>y</code> that are equal
according to <code>equals()</code> in any computation or method
invocation should produce no visible change in behavior.
</li>
<li>the class declares only final instance fields (though these may contain references
to mutable objects);</li>
<li>the class's implementations of <code>equals</code>, <code>hashCode</code>,
and <code>toString</code> compute their results solely from the values
of the class's instance fields (and the members of the objects they
reference), not from the instance's identity;</li>
<li>the class's methods treat instances as <em>freely substitutable</em>
when equal, meaning that interchanging any two instances <code>x</code> and
<code>y</code> that are equal according to <code>equals()</code> produces no
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>
<li>the class does not provide any instance creation mechanism that promises
a unique identity on each method call&mdash;in particular, any factory

This comment has been minimized.

@RogerRiggs

RogerRiggs Oct 16, 2020
Collaborator

The mdash came out as "â" in the javadoc.

This comment has been minimized.

@dansmithcode

dansmithcode Oct 16, 2020
Author Collaborator

Hmm. That's annoying. In what context? Worked okay when I did a make docs...

Are character entities supposed to work? Or is the coding convention to stick strictly to ASCII?

method's contract must allow for the possibility that if two independently-produced
instances are equal according to <code>equals()</code>, they may also be
equal according to <code>==</code>;</li>
<li>the class is final, and extends either <code>Object</code> or a hierarchy of
abstract classes that declare no instance fields or instance initializers
and whose constructors are empty.</li>
</ul>

This comment has been minimized.

@RogerRiggs

RogerRiggs Nov 2, 2020
Collaborator

Perhaps as an intro to the following points to make it clear these are about what a program should and should not do.
Should use equals(); should use explicit synchronization using lock objects or instances that are not value-based classes, etc.
Programs should not attempt to distinguish the identities of value based class instances, otherwise the result may be unpredictable.

This comment has been minimized.

@dansmithcode

dansmithcode Nov 5, 2020
Author Collaborator

I'm not totally following. This is a revision to the "When two instances" paragraph? Can you propose an alternative phrasing for the entire paragraph?

This comment has been minimized.

@RogerRiggs

RogerRiggs Nov 5, 2020
Collaborator

These two paragraphs are fine, they express the negative, not what a developer should do.

<p>A program may produce unpredictable results if it attempts to distinguish two
references to equal values of a value-based class, whether directly via reference
<p>When two instances of a value-based class are equal (according to `equals`), a program
should not attempt to distinguish between their identities, whether directly via reference
equality or indirectly via an appeal to synchronization, identity hashing,
serialization, or any other identity-sensitive mechanism. Use of such
identity-sensitive operations on instances of value-based classes may have
unpredictable effects and should be avoided.</p>
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>

This comment has been minimized.

@RogerRiggs

RogerRiggs Oct 16, 2020
Collaborator

Duplicates the requirements in the bullet list.
The bullet list seems too long and the statements are not all orthogonal, making it a bit less clear.
Suggestion:

  • Combine bullet for class is final, with the first bullet requiring final fields.
  • combing the bullets on substitutability and equality, defining substtitutability in terms of the methods.

If the paragraph above is kept, move it to before the bullet list, using the bullet list at the details that explain the more general understanding of value-based.

The last sentence is still problematic. For current usage, there is a monitor.
For Valhalla, there is no monitor and it will be a runtime error. I don't think exclusive ownership comes into it.
And it duplicates an item in the bullet list.

This comment has been minimized.

@dansmithcode

dansmithcode Oct 16, 2020
Author Collaborator

I'll revise to clarify that the bulleted list is about properties of the class declaration, while the subsequent sentences are constraints on/advice to clients. (Is there a better way to talk about clients than "a program should not..."? "Programmer", "client", "user" all kind of work, but I don't love any of them...) With that distinction in mind, I don't think the client stuff works very well until we first define what we mean by "value-based class".

I did remove what seemed to me like a redundant bullet from the original list (see my earlier comment). With what's left, each one is trying to say something distinct, although maybe some rephrasing in certain places could help.

The final class and field restrictions were combined before, but I split them because they're two very different constraints—one about extension, the other about immutability. (It's not ideal that Java uses the same keyword for both properties.) Actually, though, it works pretty well to combine the final class restriction with the superclass restriction, leaving one bullet all about subclassing. I'll do that.

The bullets that talk about equality are saying 1) the class needs an appropriate equals/hashCode/toString; 2) the class's instance methods don't have distinct behaviors for instances that are equals; 3) the class's factories do not promise distinct identities for instances that are equals. Those are three distinct things to say; I'm not seeing a clear way to combine them.

Synchronization: forget primitive classes. This is advice to current clients of value-based classes. And the advice is: don't synchronize, because you can't be sure someone else isn't doing the same on the same object. This is advice that is relevant to current clients without asking them to imagine a future in which the Object & monitor model has changed. Yet the implication is the same: don't do it.

</body>
</html>
@@ -117,13 +117,12 @@
* This difference only impacts durations measured near a leap-second and should not affect
* most applications.
* See {@link Instant} for a discussion as to the meaning of the second and time-scales.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code Duration} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -191,13 +191,12 @@
* The Java time-scale is used for all date-time classes.
* This includes {@code Instant}, {@code LocalDate}, {@code LocalTime}, {@code OffsetDateTime},
* {@code ZonedDateTime} and {@code Duration}.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code Instant} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -123,13 +123,12 @@
* For most applications written today, the ISO-8601 rules are entirely suitable.
* However, any application that makes use of historical dates, and requires them
* to be accurate will find the ISO-8601 approach unsuitable.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code LocalDate} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -119,13 +119,12 @@
* For most applications written today, the ISO-8601 rules are entirely suitable.
* However, any application that makes use of historical dates, and requires them
* to be accurate will find the ISO-8601 approach unsuitable.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code LocalDateTime} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -109,13 +109,12 @@
* The ISO-8601 calendar system is the modern civil calendar system used today
* in most of the world. This API assumes that all calendar systems use the same
* representation, this class, for time-of-day.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code LocalTime} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -111,13 +111,12 @@
* For most applications written today, the ISO-8601 rules are entirely suitable.
* However, any application that makes use of historical dates, and requires them
* to be accurate will find the ISO-8601 approach unsuitable.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code MonthDay} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -112,13 +112,12 @@
* It is intended that {@code ZonedDateTime} or {@code Instant} is used to model data
* in simpler applications. This class may be used when modeling date-time concepts in
* more detail, or when communicating to a database or in a network protocol.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code OffsetDateTime} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.
@@ -102,13 +102,12 @@
* as well as a zone offset.
* For example, the value "13:45:30.123456789+02:00" can be stored
* in an {@code OffsetTime}.
*
* <p>
* This is a <a href="{@docRoot}/java.base/java/lang/doc-files/ValueBased.html">value-based</a>
* class; use of identity-sensitive operations (including reference equality
* ({@code ==}), identity hash code, or synchronization) on instances of
* {@code OffsetTime} may have unpredictable results and should be avoided.
* 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, or unpredictable behavior may
* occur. The {@code equals} method should be used for comparisons.
*
* @implSpec
* This class is immutable and thread-safe.