RFR: 8254275: Development to revise "value-based class" & apply to wrappers [v2]

[Dan Smith]

On Wed, 14 Oct 2020 20:18:11 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

> In the text referring to ValueBase.html in many classes:
> 
> Is the "," misplaced. Should it read:
> "not use instances for synchronization or unpredictable behavior may occur."

Grammatically, it's a compound sentence. Two statements: 1) programmers should do some stuff; 2) otherwise,
unpredictable behavior. The comma helps to communicate where the "parentheses" go: (1a && 1b) || 2.

But I think you may be concerned that the "unpredictable behavior" only applies to the synchronization part of (1), not
the "interchangeable" part? It's my intent that it applies to both. If you try to distinguish between instances with
==, or do synchronization, behavior will be unpredictable (because it's possible things that were != yesterday will be
== today, per the rules about factories).

> If the comment in each class was a simple reference to ValueBased, it would be easier to maintain and there would be
> less duplication between lots of classes and the explanation in ValueBased.html.

Yes. This approach of scattering boilerplate throughout the API is not optimized for maintainability. But that's okay,
because change will not be frequent, and meanwhile it's optimized for getting readers' attention instead. I would worry
that if we stripped out all the boilerplate and just had a link, most readers wouldn't follow the link or appreciate
what it meant.

> src/java.base/share/classes/java/lang/doc-files/ValueBased.html line 43:
> 
>> 41:         <code>hashCode</code>, and <code>toString</code> which are computed
>> 42:         solely from the instance's state and not from its identity or the state
>> 43:         of any other object or variable;</li>
> 
> Quibble on pre-existing text:
> 
> If the instance state is a reference to another object, can its hashCode be included in the hashCode?
> The " not... from the state of any object" would seem to prohibit that.

Perhaps "not from its identity or the state of any other object or variable that is not part of the instance's state"?

> src/java.base/share/classes/java/lang/doc-files/ValueBased.html line 49:
> 
>> 47:         the behavior of the class's methods;
>> 48:     </li>
>> 49:     <li>perform no synchronization on an instance's intrinsic lock;</li>
> 
> "Intrinsic lock": should be a more concrete reference to some term in the JLS or JVMS.

"perform no synchronization using an instance's monitor" (aligns with terminology in JLS 17.1)

-------------

PR: https://git.openjdk.java.net/valhalla/pull/222