Re: RFR: 8252180: [JEP 390] Deprecate wrapper class constructors for removal

2020-12-07 Thread Lois Foltan
On Mon, 7 Dec 2020 15:50:55 GMT, Harold Seigel  wrote:

>> Integration of [JEP 390](https://bugs.openjdk.java.net/browse/JDK-8249100).
>> 
>> Development has been broken into 5 tasks, each with its own JBS issue:
>> - Deprecate wrapper class constructors for removal (rriggs)
>> - Revise "value-based class" & apply to wrappers (dlsmith)
>> - Define & apply annotation jdk.internal.ValueBased (rriggs)
>> - Add 'lint' warning for @ValueBased classes (sadayapalam)
>> - Diagnose synchronization on @ValueBased classes (lfoltan)
>> 
>> All changes have been previously reviewed and integrated into the [`jep390` 
>> branch](https://github.com/openjdk/valhalla/tree/jep390) of the `valhalla` 
>> repository. See the subtasks of the 5 JBS issues for these changes, 
>> including discussion and links to reviews. (Reviewers: mchung, dlsmith, 
>> jlaskey, rriggs, lfoltan, fparain, hseigel.)
>> 
>> CSRs have also been completed or are nearly complete:
>> 
>> - [JDK-8254324](https://bugs.openjdk.java.net/browse/JDK-8254324) for 
>> wrapper class constructor deprecation
>> - [JDK-8254944](https://bugs.openjdk.java.net/browse/JDK-8254944) for 
>> revisions to "value-based class"
>> - [JDK-8256023](https://bugs.openjdk.java.net/browse/JDK-8256023) for new 
>> `javac` lint warnings
>> 
>> Here's an overview of the files changed:
>> 
>> - `src/hotspot`: implementing diagnostics when `monitorenter` is applied to 
>> an instance of a class tagged with `jdk.internal.ValueBased`. This enhances 
>> previous work that produced such diagnostics for the primitive wrapper 
>> classes.
>> 
>> - `src/java.base/share/classes/java/lang`: deprecating for removal the 
>> wrapper class constructors; revising the definition of "value-based class" 
>> in `ValueBased.html` and the description used by linking classes; applying 
>> "value-based class" to the primitive wrapper classes; marking value-based 
>> classes with `@jdk.internal.ValueBased`.
>> 
>> - `src/java.base/share/classes/java/lang/constant`: no longer designating 
>> these classes as "value-based", since they rely heavily on field inheritance.
>> 
>> - `src/java.base/share/classes/java/time`: revising the description used to 
>> link to `ValueBased.html`; marking value-based classes with 
>> `@jdk.internal.ValueBased`.
>> 
>> - `src/java.base/share/classes/java/util`: revising the description used to 
>> link to `ValueBased.html`; marking value-based classes with 
>> `@jdk.internal.ValueBased`.
>> 
>> - `src/java.base/share/classes/jdk/internal`: define the 
>> `@jdk.internal.ValueBased` annotation.
>> 
>> - `src/java.management.rmi`: removing uses of wrapper class constructors.
>> 
>> - `src/java.xml`: removing uses of wrapper class constructors.
>> 
>> - `src/jdk.compiler`: implementing the `synchronization` lint category, 
>> which reports attempts to synchronize on classes and interfaces annotated 
>> with `@jdk.internal.ValueBased`.
>> 
>> - `src/jdk.incubator.foreign`: revising the description used to link to 
>> `ValueBased.html`. (Applying `@jdk.internal.ValueBased` would require a 
>> special module export, which was not deemed necessary for an incubating API.)
>> 
>> - `src/jdk.internal.vm.compiler`: suppressing `javac` deprecation and 
>> synchronization warnings in tests
>> 
>> - `src/jdk.jfr`: supplementary changes for HotSpot diagnostics
>> 
>> - `test`: testing new `javac` and HotSpot behavior; removing usages of 
>> deprecated wrapper class constructors from tests, or suppressing deprecation 
>> warnings; revising the description used to link to `ValueBased.html`.
>
> The hotspot changes look good.  In a future change, could you add additional 
> classes, such as ProcessHandle to test TestSyncOnValueBasedClassEvent.  
> Currently, it only tests primitive classes.

@hseigel thank you for the review.  I have created 
https://bugs.openjdk.java.net/browse/JDK-8257836 as an RFE to address 
additional testing.
Lois

-

PR: https://git.openjdk.java.net/jdk/pull/1636


Re: RFR: 8252180: [JEP 390] Deprecate wrapper class constructors for removal

2020-12-07 Thread Harold Seigel
On Sat, 5 Dec 2020 01:46:31 GMT, Dan Smith  wrote:

> Integration of [JEP 390](https://bugs.openjdk.java.net/browse/JDK-8249100).
> 
> Development has been broken into 5 tasks, each with its own JBS issue:
> - Deprecate wrapper class constructors for removal (rriggs)
> - Revise "value-based class" & apply to wrappers (dlsmith)
> - Define & apply annotation jdk.internal.ValueBased (rriggs)
> - Add 'lint' warning for @ValueBased classes (sadayapalam)
> - Diagnose synchronization on @ValueBased classes (lfoltan)
> 
> All changes have been previously reviewed and integrated into the [`jep390` 
> branch](https://github.com/openjdk/valhalla/tree/jep390) of the `valhalla` 
> repository. See the subtasks of the 5 JBS issues for these changes, including 
> discussion and links to reviews. (Reviewers: mchung, dlsmith, jlaskey, 
> rriggs, lfoltan, fparain, hseigel.)
> 
> CSRs have also been completed or are nearly complete:
> 
> - [JDK-8254324](https://bugs.openjdk.java.net/browse/JDK-8254324) for wrapper 
> class constructor deprecation
> - [JDK-8254944](https://bugs.openjdk.java.net/browse/JDK-8254944) for 
> revisions to "value-based class"
> - [JDK-8256023](https://bugs.openjdk.java.net/browse/JDK-8256023) for new 
> `javac` lint warnings
> 
> Here's an overview of the files changed:
> 
> - `src/hotspot`: implementing diagnostics when `monitorenter` is applied to 
> an instance of a class tagged with `jdk.internal.ValueBased`. This enhances 
> previous work that produced such diagnostics for the primitive wrapper 
> classes.
> 
> - `src/java.base/share/classes/java/lang`: deprecating for removal the 
> wrapper class constructors; revising the definition of "value-based class" in 
> `ValueBased.html` and the description used by linking classes; applying 
> "value-based class" to the primitive wrapper classes; marking value-based 
> classes with `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/java/lang/constant`: no longer designating 
> these classes as "value-based", since they rely heavily on field inheritance.
> 
> - `src/java.base/share/classes/java/time`: revising the description used to 
> link to `ValueBased.html`; marking value-based classes with 
> `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/java/util`: revising the description used to 
> link to `ValueBased.html`; marking value-based classes with 
> `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/jdk/internal`: define the 
> `@jdk.internal.ValueBased` annotation.
> 
> - `src/java.management.rmi`: removing uses of wrapper class constructors.
> 
> - `src/java.xml`: removing uses of wrapper class constructors.
> 
> - `src/jdk.compiler`: implementing the `synchronization` lint category, which 
> reports attempts to synchronize on classes and interfaces annotated with 
> `@jdk.internal.ValueBased`.
> 
> - `src/jdk.incubator.foreign`: revising the description used to link to 
> `ValueBased.html`. (Applying `@jdk.internal.ValueBased` would require a 
> special module export, which was not deemed necessary for an incubating API.)
> 
> - `src/jdk.internal.vm.compiler`: suppressing `javac` deprecation and 
> synchronization warnings in tests
> 
> - `src/jdk.jfr`: supplementary changes for HotSpot diagnostics
> 
> - `test`: testing new `javac` and HotSpot behavior; removing usages of 
> deprecated wrapper class constructors from tests, or suppressing deprecation 
> warnings; revising the description used to link to `ValueBased.html`.

The hotspot changes look good.  In a future change, could you add additional 
classes, such as ProcessHandle to test TestSyncOnValueBasedClassEvent.  
Currently, it only tests primitive classes.

-

Marked as reviewed by hseigel (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1636


Re: RFR: 8252180: [JEP 390] Deprecate wrapper class constructors for removal

2020-12-07 Thread Roger Riggs
On Sat, 5 Dec 2020 01:46:31 GMT, Dan Smith  wrote:

> Integration of [JEP 390](https://bugs.openjdk.java.net/browse/JDK-8249100).
> 
> Development has been broken into 5 tasks, each with its own JBS issue:
> - Deprecate wrapper class constructors for removal (rriggs)
> - Revise "value-based class" & apply to wrappers (dlsmith)
> - Define & apply annotation jdk.internal.ValueBased (rriggs)
> - Add 'lint' warning for @ValueBased classes (sadayapalam)
> - Diagnose synchronization on @ValueBased classes (lfoltan)
> 
> All changes have been previously reviewed and integrated into the [`jep390` 
> branch](https://github.com/openjdk/valhalla/tree/jep390) of the `valhalla` 
> repository. See the subtasks of the 5 JBS issues for these changes, including 
> discussion and links to reviews. (Reviewers: mchung, dlsmith, jlaskey, 
> rriggs, lfoltan, fparain, hseigel.)
> 
> CSRs have also been completed or are nearly complete:
> 
> - [JDK-8254324](https://bugs.openjdk.java.net/browse/JDK-8254324) for wrapper 
> class constructor deprecation
> - [JDK-8254944](https://bugs.openjdk.java.net/browse/JDK-8254944) for 
> revisions to "value-based class"
> - [JDK-8256023](https://bugs.openjdk.java.net/browse/JDK-8256023) for new 
> `javac` lint warnings
> 
> Here's an overview of the files changed:
> 
> - `src/hotspot`: implementing diagnostics when `monitorenter` is applied to 
> an instance of a class tagged with `jdk.internal.ValueBased`. This enhances 
> previous work that produced such diagnostics for the primitive wrapper 
> classes.
> 
> - `src/java.base/share/classes/java/lang`: deprecating for removal the 
> wrapper class constructors; revising the definition of "value-based class" in 
> `ValueBased.html` and the description used by linking classes; applying 
> "value-based class" to the primitive wrapper classes; marking value-based 
> classes with `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/java/lang/constant`: no longer designating 
> these classes as "value-based", since they rely heavily on field inheritance.
> 
> - `src/java.base/share/classes/java/time`: revising the description used to 
> link to `ValueBased.html`; marking value-based classes with 
> `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/java/util`: revising the description used to 
> link to `ValueBased.html`; marking value-based classes with 
> `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/jdk/internal`: define the 
> `@jdk.internal.ValueBased` annotation.
> 
> - `src/java.management.rmi`: removing uses of wrapper class constructors.
> 
> - `src/java.xml`: removing uses of wrapper class constructors.
> 
> - `src/jdk.compiler`: implementing the `synchronization` lint category, which 
> reports attempts to synchronize on classes and interfaces annotated with 
> `@jdk.internal.ValueBased`.
> 
> - `src/jdk.incubator.foreign`: revising the description used to link to 
> `ValueBased.html`. (Applying `@jdk.internal.ValueBased` would require a 
> special module export, which was not deemed necessary for an incubating API.)
> 
> - `src/jdk.internal.vm.compiler`: suppressing `javac` deprecation and 
> synchronization warnings in tests
> 
> - `src/jdk.jfr`: supplementary changes for HotSpot diagnostics
> 
> - `test`: testing new `javac` and HotSpot behavior; removing usages of 
> deprecated wrapper class constructors from tests, or suppressing deprecation 
> warnings; revising the description used to link to `ValueBased.html`.

For the core libraries parts looks ok.

-

Marked as reviewed by rriggs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1636


Re: RFR: 8252180: [JEP 390] Deprecate wrapper class constructors for removal

2020-12-04 Thread Dan Smith
On Sat, 5 Dec 2020 01:46:31 GMT, Dan Smith  wrote:

> Integration of [JEP 390](https://bugs.openjdk.java.net/browse/JDK-8249100).
> 
> Development has been broken into 5 tasks, each with its own JBS issue:
> - Deprecate wrapper class constructors for removal
> - Revise "value-based class" & apply to wrappers
> - Define & apply annotation jdk.internal.ValueBased
> - Add 'lint' warning for @ValueBased classes
> - Diagnose synchronization on @ValueBased classes
> 
> All changes have been previously reviewed and integrated into the [`jep390` 
> branch](https://github.com/openjdk/valhalla/tree/jep390) of the `valhalla` 
> repository. See the subtasks of the 5 JBS issues for these changes, including 
> discussion and links to reviews. (Reviewers: mchung, dlsmith, jlaskey, 
> rriggs, lfoltan, fparain, hseigel.)
> 
> CSRs have also been completed or are nearly complete:
> 
> - [JDK-8254324](https://bugs.openjdk.java.net/browse/JDK-8254324) for wrapper 
> class constructor deprecation
> - [JDK-8254944](https://bugs.openjdk.java.net/browse/JDK-8254944) for 
> revisions to "value-based class"
> - [JDK-8256023](https://bugs.openjdk.java.net/browse/JDK-8256023) for new 
> `javac` lint warnings
> 
> Here's an overview of the files changed:
> 
> - `src/hotspot`: implementing diagnostics when `monitorenter` is applied to 
> an instance of a class tagged with `jdk.internal.ValueBased`. This enhances 
> previous work that produced such diagnostics for the primitive wrapper 
> classes.
> 
> - `src/java.base/share/classes/java/lang`: deprecating for removal the 
> wrapper class constructors; revising the definition of "value-based class" in 
> `ValueBased.html` and the description used by linking classes; applying 
> "value-based class" to the primitive wrapper classes; marking value-based 
> classes with `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/java/lang/constant`: no longer designating 
> these classes as "value-based", since they rely heavily on field inheritance.
> 
> - `src/java.base/share/classes/java/time`: revising the description used to 
> link to `ValueBased.html`; marking value-based classes with 
> `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/java/util`: revising the description used to 
> link to `ValueBased.html`; marking value-based classes with 
> `@jdk.internal.ValueBased`.
> 
> - `src/java.base/share/classes/jdk/internal`: define the 
> `@jdk.internal.ValueBased` annotation.
> 
> - `src/java.management.rmi`: removing uses of wrapper class constructors.
> 
> - `src/java.xml`: removing uses of wrapper class constructors.
> 
> - `src/jdk.compiler`: implementing the `synchronization` lint category, which 
> reports attempts to synchronize on classes and interfaces annotated with 
> `@jdk.internal.ValueBased`.
> 
> - `src/jdk.incubator.foreign`: revising the description used to link to 
> `ValueBased.html`. (Applying `@jdk.internal.ValueBased` would require a 
> special module export, which was not deemed necessary for an incubating API.)
> 
> - `src/jdk.internal.vm.compiler`: suppressing `javac` deprecation and 
> synchronization warnings in tests
> 
> - `src/jdk.jfr`: supplementary changes for HotSpot diagnostics
> 
> - `test`: testing new `javac` and HotSpot behavior; removing usages of 
> deprecated wrapper class constructors from tests, or suppressing deprecation 
> warnings; revising the description used to link to `ValueBased.html`.

I've updated the description to provide an overview for those unfamiliar with 
this work. Reviewers are welcome, including those who have previously reviewed 
a subset of these changes.

-

PR: https://git.openjdk.java.net/jdk/pull/1636