On 08/05/15 13:13, Pavel Rappo wrote:
Here's the latest update:
http://cr.openjdk.java.net/~prappo/8029689/webrev.01/
The source changes look good to me Pavel.
Just a few minor comments on the test:
1) There is a trivial System.out on L143 that could be removed.
2) L157, is this referring to computeIfAbsent? I think a small
comment about concurrency would be sufficient.
-Chris.
-Pavel
On 21 Apr 2015, at 09:08, Chris Hegarty <[email protected]> wrote:
On 21 Apr 2015, at 03:00, David Holmes <[email protected]> wrote:
On 21/04/2015 1:24 AM, Chris Hegarty wrote:
On 20/04/15 16:17, Lance Andersen wrote:
Hi Pavel,
So we are just documenting/clarifying the current behavior from what I
can tell from the change?
Looking at the testcase, it passes with the current JDK 9 ( and 8 ), so
this is just documenting existing behavior.
Right. The assumption is that original authors overlooked the fact that the
@exception/@throws for unchecked exceptions would not automatically be
inherited by subclasses, and that in fact those subclasses (in this case)
should indeed have inherited the same preconditions from the parent.
All subclasses of java.io.Reader are documenting existing behaviour ( such
documentation of uncheck exceptions was not the norm when these classes were
originally written, but seems reasonable in this case ). But
java.io.Reader.read(String,int,int) is abstract, and as such is having
additional specification added ( which should be fine ).
The only question is about third party implementations of Reader ( or other non
Java SE implementations in the JDK), which may need to be amended to confirm to
this “new” bounds check. Again, I think this should be fine, and seems in line
with the evolution policy of core libraries.
-Chris.
David
If so, this looks OK assuming you have an approved CCC? The test
seems fine.
A CCC will be needed, and should be submitted after successful review.
I am assuming there should not be any issues here but would be good to
hear from others on this change as well
Right. We do this ( clarify spec from existing behavior) in other areas
too.
Given that this is the current behavior of existing platform subtypes,
then this change is only specifying existing behavior. Since there are
no implementation changes, then Third party Reader subtypes will
continue to function as before, but they may need to be updated at some
point in the future.
-Chris.
Best
Lance
On Apr 20, 2015, at 11:10 AM, Pavel Rappo <[email protected]> wrote:
Hi everyone,
Could you please review my change for JDK-8029689
http://cr.openjdk.java.net/~prappo/8029689/webrev.00/
-------------------------------------------------------------------------------
There is a long-standing issue when platform implementations of
java.io.Reader
throw IndexOutOfBoundsException for bounds checks from inherited
java.io.Reader.read(char[], int, int) method though java.io.Reader
itself does
not specify this situation.
Suggested solution is to update the contract of
java.io.Reader.read(char[], int,
int) and its publicly exported descendants to capture the implied
preconditions
for reading range and the array size.
Given that throwing IOBE in this situation is a de facto standard,
this change
won't bring any kind of incompatibility, though to stay compliant 3rd
party
implementations may need to be updated.
-Pavel
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[email protected]
