RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Brian Burkhalter-3
8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

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

Commit messages:
 - 8258444: Fix trailing whitesoace
 - 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

Changes: https://git.openjdk.java.net/jdk/pull/2680/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2680&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258444
  Stats: 156 lines in 9 files changed: 41 ins; 57 del; 58 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2680.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2680/head:pull/2680

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Brian Burkhalter-3
On Mon, 22 Feb 2021 23:27:19 GMT, Brian Burkhalter <[hidden email]> wrote:

> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

This change proposes to use {@inheritDoc} to improve consistency of the specification of `Reader.read(char[],in,in)` in `Reader` and its subclasses. In the course of this, some bounds checks are replaced with `Objects.checkFromIndexSize()`.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Alan Bateman-2
In reply to this post by Brian Burkhalter-3
On Mon, 22 Feb 2021 23:27:19 GMT, Brian Burkhalter <[hidden email]> wrote:

> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

Marked as reviewed by alanb (Reviewer).

src/java.base/share/classes/java/io/InputStreamReader.java line 167:

> 165:      * {@inheritDoc}
> 166:      */
> 167:     public int read(char[] cbuf, int off, int len) throws IOException {

IOOBE is unchecked, are you sure it gets inherited into the sub-class here?

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Brian Burkhalter-3
On Tue, 23 Feb 2021 08:48:16 GMT, Alan Bateman <[hidden email]> wrote:

>> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides
>
> src/java.base/share/classes/java/io/InputStreamReader.java line 167:
>
>> 165:      * {@inheritDoc}
>> 166:      */
>> 167:     public int read(char[] cbuf, int off, int len) throws IOException {
>
> IOOBE is unchecked, are you sure it gets inherited into the sub-class here?

It does not due to https://bugs.openjdk.java.net/browse/JDK-8157677.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Alan Bateman-2
On Tue, 23 Feb 2021 16:19:41 GMT, Brian Burkhalter <[hidden email]> wrote:

>> src/java.base/share/classes/java/io/InputStreamReader.java line 167:
>>
>>> 165:      * {@inheritDoc}
>>> 166:      */
>>> 167:     public int read(char[] cbuf, int off, int len) throws IOException {
>>
>> IOOBE is unchecked, are you sure it gets inherited into the sub-class here?
>
> It does not due to https://bugs.openjdk.java.net/browse/JDK-8157677.

The long standard behavior is that javadoc doesn't copy the  throws of unchecked exceptions, instead we've had to declare the throws and use inheritDoc. I assume you'll need to do that here.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Brian Burkhalter-3
On Tue, 23 Feb 2021 16:23:10 GMT, Alan Bateman <[hidden email]> wrote:

>> src/java.base/share/classes/java/io/InputStreamReader.java line 167:
>>
>>> 165:      * {@inheritDoc}
>>> 166:      */
>>> 167:     public int read(char[] cbuf, int off, int len) throws IOException {
>>
>> IOOBE is unchecked, are you sure it gets inherited into the sub-class here?
>
> The long standard behavior is that javadoc doesn't copy the  throws of unchecked exceptions, instead we've had to declare the throws and use inheritDoc. I assume you'll need to do that here.

It does not and there is a javadoc issue already on file about this.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Brian Burkhalter-3
In reply to this post by Brian Burkhalter-3
> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:

  8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2680/files
  - new: https://git.openjdk.java.net/jdk/pull/2680/files/c96c80f0..687ad124

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2680&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2680&range=00-01

  Stats: 2 lines in 2 files changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2680.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2680/head:pull/2680

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Alan Bateman-2
On Tue, 23 Feb 2021 17:42:58 GMT, Brian Burkhalter <[hidden email]> wrote:

>> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>
>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader

Marked as reviewed by alanb (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Brian Burkhalter-3
On Tue, 23 Feb 2021 18:00:33 GMT, Alan Bateman <[hidden email]> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader
>
> Marked as reviewed by alanb (Reviewer).

CSR filed: https://bugs.openjdk.java.net/browse/JDK-8262262.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Roger Riggs-2
In reply to this post by Brian Burkhalter-3
On Tue, 23 Feb 2021 17:42:58 GMT, Brian Burkhalter <[hidden email]> wrote:

>> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>
>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader

Nice cleanup.

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

Marked as reviewed by rriggs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Daniel Fuchs-2
In reply to this post by Brian Burkhalter-3
On Tue, 23 Feb 2021 17:42:58 GMT, Brian Burkhalter <[hidden email]> wrote:

>> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>
>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader

src/java.base/share/classes/java/io/PushbackReader.java line 98:

> 96:      * {@inheritDoc}
> 97:      */
> 98:     public int read(char[] cbuf, int off, int len) throws IOException {

Shouldn't you add

 * @throws IndexOutOfBoundException {@inheritDoc}
 * @throws IOException {@inheritDoc}

here as well? IIRC the global {@inheritDoc} will not add them.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Brian Burkhalter-3
On Tue, 23 Feb 2021 22:12:38 GMT, Daniel Fuchs <[hidden email]> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader
>
> src/java.base/share/classes/java/io/PushbackReader.java line 98:
>
>> 96:      * {@inheritDoc}
>> 97:      */
>> 98:     public int read(char[] cbuf, int off, int len) throws IOException {
>
> Shouldn't you add
>
>  * @throws IndexOutOfBoundException {@inheritDoc}
>  * @throws IOException {@inheritDoc}
>
> here as well? IIRC the global {@inheritDoc} will not add them.

No. In this case the specification no longer appears in the main method summary but rather under `Methods declared in class java.io.FilterReader` which has a full spec.

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides [v2]

Brian Burkhalter-3
In reply to this post by Alan Bateman-2
On Tue, 23 Feb 2021 18:00:33 GMT, Alan Bateman <[hidden email]> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8258444: Explcitly inherit IOOBE in {Filter,InputStream}Reader
>
> Marked as reviewed by alanb (Reviewer).

@AlanBateman, @dfuch, @RogerRiggs thanks for the reviews here and @jddarcy for approving the CSR!

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

PR: https://git.openjdk.java.net/jdk/pull/2680
Reply | Threaded
Open this post in threaded view
|

Integrated: 8258444: Clean up specifications of java.io.Reader.read(char[], int, int) in subclass overrides

Brian Burkhalter-3
In reply to this post by Brian Burkhalter-3
On Mon, 22 Feb 2021 23:27:19 GMT, Brian Burkhalter <[hidden email]> wrote:

> 8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

This pull request has now been integrated.

Changeset: 5a9b7010
Author:    Brian Burkhalter <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/5a9b7010
Stats:     156 lines in 9 files changed: 41 ins; 55 del; 60 mod

8258444: Clean up specifications of java.io.Reader.read(char[],int,int) in subclass overrides

Reviewed-by: alanb, rriggs

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

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