Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased [v2]
On Fri, 19 Nov 2021 20:47:40 GMT, Roger Riggs wrote: >> The `jdk.internal.ValueBased` annotation was incorrectly applied to >> subclasses of java.util.AbstractMap. >> [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) >> requires that supertypes have no instance fields; AbstractMap has instance >> fields keySet and values. >> >> Remove the internal @ValueBased annotation for subclasses of AbstractMap >> including: >> AbstractImmutableMap, Map1, and MapN. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Added comment explaining why immutable Maps are not 'ValueBased' Marked as reviewed by smarks (Reviewer). Yes, we want to leave the "value-based" stipulation in the specification, even if the ValueBased annotation is removed. In the future we might be able to remove the dependency on `AbstractMap` and thus avoid the mutable fields in the unmodifiable collections. Or maybe `AbstractMap` should lose them entirely. The `keySet` and `values` collections are fairly small objects themselves, so caching them might not be much benefit in the first place. - PR: https://git.openjdk.java.net/jdk/pull/6480
Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased [v2]
On Sun, 21 Nov 2021 11:15:53 GMT, Jens Li wrote: > Should this change also update the Javadoc on `java.util.Map` and remove the > statement that the return values of `Map#of` are value-based? > This PR is only correcting the application of the internal use annotation to some implementation classes. The descriptions of unmodifiable maps identify the conditions the clients of Map.of may and may not assume about the returned instances including those related to identity. - PR: https://git.openjdk.java.net/jdk/pull/6480
Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased [v2]
On Fri, 19 Nov 2021 20:47:40 GMT, Roger Riggs wrote: >> The `jdk.internal.ValueBased` annotation was incorrectly applied to >> subclasses of java.util.AbstractMap. >> [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) >> requires that supertypes have no instance fields; AbstractMap has instance >> fields keySet and values. >> >> Remove the internal @ValueBased annotation for subclasses of AbstractMap >> including: >> AbstractImmutableMap, Map1, and MapN. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Added comment explaining why immutable Maps are not 'ValueBased' Should this change also update the Javadoc on `java.util.Map` and remove the statement that the return values of `Map#of` are value-based? See the [documentation on `Map`](https://github.com/openjdk/jdk/blob/42fce0394b80df0c9f46f826f624d3714274c5bf/src/java.base/share/classes/java/util/Map.java). The documentation on `Map` refers to the [documentation for value-based classes](https://github.com/openjdk/jdk/blob/42fce0394b80df0c9f46f826f624d3714274c5bf/src/java.base/share/classes/java/lang/doc-files/ValueBased.html). Or maybe the documentation on `Map` should say that `Map#of` _might_ return value-based objects? In that case the implementation have the possibility to change in the future. - PR: https://git.openjdk.java.net/jdk/pull/6480
Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased [v2]
On Fri, 19 Nov 2021 20:47:40 GMT, Roger Riggs wrote: >> The `jdk.internal.ValueBased` annotation was incorrectly applied to >> subclasses of java.util.AbstractMap. >> [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) >> requires that supertypes have no instance fields; AbstractMap has instance >> fields keySet and values. >> >> Remove the internal @ValueBased annotation for subclasses of AbstractMap >> including: >> AbstractImmutableMap, Map1, and MapN. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Added comment explaining why immutable Maps are not 'ValueBased' Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6480
Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased [v2]
> The `jdk.internal.ValueBased` annotation was incorrectly applied to > subclasses of java.util.AbstractMap. > [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) > requires that supertypes have no instance fields; AbstractMap has instance > fields keySet and values. > > Remove the internal @ValueBased annotation for subclasses of AbstractMap > including: > AbstractImmutableMap, Map1, and MapN. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Added comment explaining why immutable Maps are not 'ValueBased' - Changes: - all: https://git.openjdk.java.net/jdk/pull/6480/files - new: https://git.openjdk.java.net/jdk/pull/6480/files/c1f579ea..42fce039 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6480=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6480=00-01 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6480.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6480/head:pull/6480 PR: https://git.openjdk.java.net/jdk/pull/6480
Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased
On Fri, 19 Nov 2021 15:50:47 GMT, Roger Riggs wrote: > The `jdk.internal.ValueBased` annotation was incorrectly applied to > subclasses of java.util.AbstractMap. > [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) > requires that supertypes have no instance fields; AbstractMap has instance > fields keySet and values. > > Remove the internal @ValueBased annotation for subclasses of AbstractMap > including: > AbstractImmutableMap, Map1, and MapN. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6480
Re: RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased
On Fri, 19 Nov 2021 15:50:47 GMT, Roger Riggs wrote: > The `jdk.internal.ValueBased` annotation was incorrectly applied to > subclasses of java.util.AbstractMap. > [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) > requires that supertypes have no instance fields; AbstractMap has instance > fields keySet and values. > > Remove the internal @ValueBased annotation for subclasses of AbstractMap > including: > AbstractImmutableMap, Map1, and MapN. It may be helpful to add a comment why these classes are not value-based classes. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6480
RFR: 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased
The `jdk.internal.ValueBased` annotation was incorrectly applied to subclasses of java.util.AbstractMap. [ValueBased](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/doc-files/ValueBased.html) requires that supertypes have no instance fields; AbstractMap has instance fields keySet and values. Remove the internal @ValueBased annotation for subclasses of AbstractMap including: AbstractImmutableMap, Map1, and MapN. - Commit messages: - 8272042: java.util.ImmutableCollections$Map1 and MapN should not be @ValueBased Changes: https://git.openjdk.java.net/jdk/pull/6480/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6480=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272042 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6480.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6480/head:pull/6480 PR: https://git.openjdk.java.net/jdk/pull/6480
