JDK 9 RFR(s): 8173152: Wrong wording in Comparator.compare() method spec

[Stuart Marks]

Hi all,

Please review this small javadoc fix to correct the wording in 
Comparator.compare(). There's a sentence defining the sgn() notation, that says 
"In the foregoing description" but it occurs *before* the actual use of sgn(). 
I've moved this to the end of the method spec.


This also makes the Comparator.compare() method spec very similar to the 
Comparable.compareTo() method spec.


Thanks,

s'marks


# HG changeset patch
# User smarks
# Date 1491530800 25200
#  Thu Apr 06 19:06:40 2017 -0700
# Node ID 1f08b6bc20078e07f7264e449a334234906d46e5
# Parent  7c72114a555820da956c81364ae7ba30698aa799
8173152: Wrong wording in Comparator.compare() method spec
Reviewed-by: XXX

diff -r 7c72114a5558 -r 1f08b6bc2007 
src/java.base/share/classes/java/util/Comparator.java
--- a/src/java.base/share/classes/java/util/Comparator.java	Fri Mar 31 14:21:21 
2017 -0700
+++ b/src/java.base/share/classes/java/util/Comparator.java	Thu Apr 06 19:06:40 
2017 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -112,12 +112,6 @@
  * zero, or a positive integer as the first argument is less than, equal
  * to, or greater than the second.
  *
- * In the foregoing description, the notation
- * {@code sgn(}expression{@code )} designates the mathematical
- * signum function, which is defined to return one of {@code -1},
- * {@code 0}, or {@code 1} according to whether the value of
- * expression is negative, zero or positive.
- *
  * The implementor must ensure that {@code sgn(compare(x, y)) ==
  * -sgn(compare(y, x))} for all {@code x} and {@code y}.  (This
  * implies that {@code compare(x, y)} must throw an exception if and only
@@ -135,7 +129,13 @@
  * {@code (compare(x, y)==0) == (x.equals(y))}.  Generally speaking,
  * any comparator that violates this condition should clearly indicate
  * this fact.  The recommended language is "Note: this comparator
- * imposes orderings that are inconsistent with equals."
+ * imposes orderings that are inconsistent with equals."
+ *
+ * In the foregoing description, the notation
+ * {@code sgn(}expression{@code )} designates the mathematical
+ * signum function, which is defined to return one of {@code -1},
+ * {@code 0}, or {@code 1} according to whether the value of
+ * expression is negative, zero or positive.
  *
  * @param o1 the first object to be compared.
  * @param o2 the second object to be compared.

Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest

[Naoto Sato]

Hi Brent, thank you for the review.

On 4/6/17 1:08 PM, Brent Christian wrote:

Hi, Naoto

On 4/5/17 2:14 PM, Naoto Sato wrote:

I revised the test case not to rely on shell script.


Yay!  Hopefully this can also happen sometime for JDK 9+.


Sure. Will work on it.




http://cr.openjdk.java.net/~naoto/816/webrev.01/


Looks fine to me, Naoto.  A few comments:

* I presume additional @bug values will be added as other fixes are
backported (e.g. 8054214).


Good point. Will add those bug ids on push.



* On

  73 Path dst = Paths.get("testjava").toAbsolutePath();

This places "dst" within the scratch directory, then?  (I thought there
was a jtreg system property for the scratch directory, but I can't find
it so I think I'm mis-remembering).

I agree with letting jtreg take care of cleaning up "scratch".


Yes, "dst" will be "scratch". Not aware of any system property to 
designate the output directory, other than "-w" command option to 
specify the JTwork dir.




If anything goes wrong with copying of the JDK (e.g. full disk),
hopefully it would be easy to diagnose, with an IOException with a full
stack trace.

* Have you confirmed (if it's practical to do so) that this test fails
when expected (detects a bug)?


Yes. Without those backports, the test will fail as expected.

Naoto



Thanks,
-Brent


On 3/30/17 2:10 PM, Naoto Sato wrote:

Hello,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-816

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/816/webrev.00/

This is for backporting fixes for JapaneseEra related issues (8054214,
8173423). The original fixes in JDK9 included a test case,
SupplementalJapaneseEraTest which is intended for the system property
testing. The above proposed fix intends to adapt that test case into
JDK8, where calendars.properties file is used instead of the system
property. The test is pretty much identical to JDK9's. The difference is
to deal with the calendars.properties file and removed some
non-applicable cases in JDK8.

Naoto

Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest

[Brent Christian]

Hi, Naoto

On 4/5/17 2:14 PM, Naoto Sato wrote:

I revised the test case not to rely on shell script.


Yay!  Hopefully this can also happen sometime for JDK 9+.


http://cr.openjdk.java.net/~naoto/816/webrev.01/


Looks fine to me, Naoto.  A few comments:

* I presume additional @bug values will be added as other fixes are 
backported (e.g. 8054214).


* On

  73 Path dst = Paths.get("testjava").toAbsolutePath();

This places "dst" within the scratch directory, then?  (I thought there 
was a jtreg system property for the scratch directory, but I can't find 
it so I think I'm mis-remembering).


I agree with letting jtreg take care of cleaning up "scratch".

If anything goes wrong with copying of the JDK (e.g. full disk), 
hopefully it would be easy to diagnose, with an IOException with a full 
stack trace.


* Have you confirmed (if it's practical to do so) that this test fails 
when expected (detects a bug)?


Thanks,
-Brent


On 3/30/17 2:10 PM, Naoto Sato wrote:

Hello,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-816

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/816/webrev.00/

This is for backporting fixes for JapaneseEra related issues (8054214,
8173423). The original fixes in JDK9 included a test case,
SupplementalJapaneseEraTest which is intended for the system property
testing. The above proposed fix intends to adapt that test case into
JDK8, where calendars.properties file is used instead of the system
property. The test is pretty much identical to JDK9's. The difference is
to deal with the calendars.properties file and removed some
non-applicable cases in JDK8.

Naoto

Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest

[Naoto Sato]

Thanks for reviewing, Roger.

On 4/6/17 8:39 AM, Roger Riggs wrote:

Hi Naoto,

Thanks for replacing the shell script with Java code.

Given the size of the JDK, I'd suggest removing the copy at the end of
the test
unless you can rely on jtreg to remove it promptly.


I was thinking about using @AfterTest to do it, but then realized that 
jtreg automatically cleans up the "scratch" directory after the test, 
where the test JDK is copied into. Here is the quote from jtreg's page [1]:


---
jtreg will automatically clean up any files written in the scratch 
directory. In general, you should not clean these files up within the 
test because they may provide infomation useful to diagnose a test 
failure, should that be necessary.

---

Naoto

[1] http://openjdk.java.net/jtreg/writetests.html



The rest looks fine,  Roger


On 4/5/2017 5:14 PM, Naoto Sato wrote:

I revised the test case not to rely on shell script. Please review.

http://cr.openjdk.java.net/~naoto/816/webrev.01/

Naoto

On 3/30/17 2:10 PM, Naoto Sato wrote:

Hello,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-816

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/816/webrev.00/

This is for backporting fixes for JapaneseEra related issues (8054214,
8173423). The original fixes in JDK9 included a test case,
SupplementalJapaneseEraTest which is intended for the system property
testing. The above proposed fix intends to adapt that test case into
JDK8, where calendars.properties file is used instead of the system
property. The test is pretty much identical to JDK9's. The difference is
to deal with the calendars.properties file and removed some
non-applicable cases in JDK8.

Naoto

Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest

[Roger Riggs]

Hi Naoto,

Thanks for replacing the shell script with Java code.

Given the size of the JDK, I'd suggest removing the copy at the end of 
the test

unless you can rely on jtreg to remove it promptly.

The rest looks fine,  Roger


On 4/5/2017 5:14 PM, Naoto Sato wrote:

I revised the test case not to rely on shell script. Please review.

http://cr.openjdk.java.net/~naoto/816/webrev.01/

Naoto

On 3/30/17 2:10 PM, Naoto Sato wrote:

Hello,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-816

The proposed change is located at:

http://cr.openjdk.java.net/~naoto/816/webrev.00/

This is for backporting fixes for JapaneseEra related issues (8054214,
8173423). The original fixes in JDK9 included a test case,
SupplementalJapaneseEraTest which is intended for the system property
testing. The above proposed fix intends to adapt that test case into
JDK8, where calendars.properties file is used instead of the system
property. The test is pretty much identical to JDK9's. The difference is
to deal with the calendars.properties file and removed some
non-applicable cases in JDK8.

Naoto

Re: [PATCH] 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics

[Chris Dennis]

# HG changeset patch
# User chris_dennis
# Date 1491485015 14400
#  Thu Apr 06 09:23:35 2017 -0400
# Node ID d789970b8393032457885e739d76919f714bbd50
# Parent  c0aecf84349c97f4241eab01f7bbfb7660d51be1
8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics

diff --git a/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java 
b/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java
--- a/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java
+++ b/src/java.base/share/classes/java/util/DoubleSummaryStatistics.java
@@ -76,6 +76,47 @@
 public DoubleSummaryStatistics() { }
 
 /**
+ * Construct a non-empty instance with the supplied min, max, count, and 
sum.
+ *
+ * If {@code count} is zero then the remaining parameters are ignored
+ * and an empty instance is constructed.
+ * If the arguments are inconsistent then an {@code 
IllegalArgumentException}
+ * is thrown.  The necessary consistent argument conditions are:
+ * 
+ *   {@code count}  0
+ *   {@code min}  {@code max}
+ *   ({@code count}  {@code max})  {@code sum}
+ *   ({@code count}  {@code min})  {@code sum}
+ * 
+ * This enforcement of argument consistency means that the retrieved set of
+ * values from a {@code DoubleSummaryStatistics} instance may not be a 
legal
+ * set of arguments for this constructor due to arithmetic overflow in the
+ * original instance.
+ *
+ * @param min the minimum value
+ * @param max the maximum value
+ * @param count the count of values
+ * @param sum the sum of all values
+ * @throws IllegalArgumentException if the arguments are inconsistent
+ */
+public DoubleSummaryStatistics(double min, double max, long count, double 
sum) throws IllegalArgumentException {
+if (count < 0L) {
+throw new IllegalArgumentException("Negative count value");
+} else if (count > 0L) {
+if (min > max) throw new IllegalArgumentException("Minimum greater 
than maximum");
+if (count * max < sum) throw new IllegalArgumentException("Maximum 
inconsistent with sum and count");
+if (count * min > sum) throw new IllegalArgumentException("Minimum 
inconsistent with sum and count");
+
+this.count = count;
+this.sum = sum;
+this.simpleSum = sum;
+this.sumCompensation = 0.0d;
+this.min = min;
+this.max = max;
+}
+}
+
+/**
  * Records another value into the summary information.
  *
  * @param value the input value
diff --git a/src/java.base/share/classes/java/util/IntSummaryStatistics.java 
b/src/java.base/share/classes/java/util/IntSummaryStatistics.java
--- a/src/java.base/share/classes/java/util/IntSummaryStatistics.java
+++ b/src/java.base/share/classes/java/util/IntSummaryStatistics.java
@@ -27,6 +27,9 @@
 import java.util.function.IntConsumer;
 import java.util.stream.Collector;
 
+import static java.lang.Math.multiplyExact;
+import static java.lang.Math.multiplyFull;
+
 /**
  * A state object for collecting statistics such as count, min, max, sum, and
  * average.
@@ -76,6 +79,49 @@
 public IntSummaryStatistics() { }
 
 /**
+ * Construct a non-empty instance with the supplied min, max, count, and 
sum.
+ *
+ * If {@code count} is zero then the remaining parameters are ignored
+ * and an empty instance is constructed.
+ * If the arguments are inconsistent then an {@code 
IllegalArgumentException}
+ * is thrown.  The necessary consistent argument conditions are:
+ * 
+ *   {@code count}  0
+ *   {@code min}  {@code max}
+ *   ({@code count}  {@code max})  {@code sum}
+ *   ({@code count}  {@code min})  {@code sum}
+ * 
+ * This enforcement of argument consistency means that the retrieved set of
+ * values from an {@code IntSummaryStatistics} instance may not be a legal
+ * set of arguments for this constructor due to arithmetic overflow in the
+ * original instance.
+ *
+ * @param min the minimum value
+ * @param max the maximum value
+ * @param count the count of values
+ * @param sum the sum of all values
+ * @throws IllegalArgumentException if the arguments are inconsistent
+ */
+public IntSummaryStatistics(int min, int max, long count, long sum) throws 
IllegalArgumentException {
+if (count < 0L) {
+throw new IllegalArgumentException("Negative count value");
+} else if (count > 0L) {
+if (min > max) throw new IllegalArgumentException("Minimum greater 
than maximum");
+try {
+if (multiplyExact(count, max) < sum) throw new 
IllegalArgumentException("Maximum inconsistent with sum and count");
+} catch (ArithmeticException e) {}
+try {
+if (multiplyExact(count, min) > sum) throw new 
IllegalArgumentException("Minimum 

Re: RFR 9: 8165641 : Deprecate Object.finalize

[Roger Riggs]

Thanks Mandy,  I'll put you down as a reviewer for the fix. (8165641 was 
pushed)



diff --git a/src/java.base/share/classes/java/lang/Object.java 
b/src/java.base/share/classes/java/lang/Object.java

--- a/src/java.base/share/classes/java/lang/Object.java
+++ b/src/java.base/share/classes/java/lang/Object.java
@@ -593,7 +593,7 @@ public class Object {
  * finalization if it is no longer necessary; and no ordering is 
specified

  * among calls to {@code finalize} methods of different objects.
  * Furthermore, there are no guarantees regarding the timing of 
finalization.

- * The {@code finalize} method might be called on an finalizable object
+ * The {@code finalize} method might be called on a finalizable object
  * only after an indefinite delay, if at all.
  *
  * Classes whose instances hold non-heap resources should provide 
a method




[1] https://bugs.openjdk.java.net/browse/JDK-8178154



On 4/5/2017 6:15 PM, Mandy Chung wrote:

On Apr 3, 2017, at 7:44 AM, Roger Riggs  wrote:

[1] http://cr.openjdk.java.net/~rriggs/webrev-finalize-deprecate-8165641


Typo: s/an finalizable/a finalizable/

+ * The {@code finalize} method might be called on an finalizable object

Otherwise, looks good.  No need for a new webrev.

Mandy

Re: RFR 9: 8165641 : Deprecate Object.finalize

[Mandy Chung]

+1

Mandy

> On Apr 6, 2017, at 7:44 AM, Roger Riggs  wrote:
> 
> Thanks Mandy,  I'll put you down as a reviewer for the fix.  (8165641 was 
> pushed)
> 
> 
> diff --git a/src/java.base/share/classes/java/lang/Object.java 
> b/src/java.base/share/classes/java/lang/Object.java
> --- a/src/java.base/share/classes/java/lang/Object.java
> +++ b/src/java.base/share/classes/java/lang/Object.java
> @@ -593,7 +593,7 @@ public class Object {
>   * finalization if it is no longer necessary; and no ordering is 
> specified
>   * among calls to {@code finalize} methods of different objects.
>   * Furthermore, there are no guarantees regarding the timing of 
> finalization.
> - * The {@code finalize} method might be called on an finalizable object
> + * The {@code finalize} method might be called on a finalizable object
>   * only after an indefinite delay, if at all.
>   *
>   * Classes whose instances hold non-heap resources should provide a 
> method
> 
> 
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8178154 
> 
> 
> 
> 
> On 4/5/2017 6:15 PM, Mandy Chung wrote:
>>> On Apr 3, 2017, at 7:44 AM, Roger Riggs  
>>>  wrote:
>>> 
>>> [1] http://cr.openjdk.java.net/~rriggs/webrev-finalize-deprecate-8165641 
>>> 
>> 
>> Typo: s/an finalizable/a finalizable/
>> 
>> + * The {@code finalize} method might be called on an finalizable object
>> 
>> Otherwise, looks good.  No need for a new webrev.
>> 
>> Mandy
> 

Re: [PATCH] 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics

[Jonathan Bluett-Duncan]

Hi Chris,

Unfortunately the patch you sent (in what I presume was an attachment) is
missing. I believe the OpenJDK mailing list servers intentionally strip out
attachments in all emails, which seems to be at odds with the advice given
in http://openjdk.java.net/contribute/. (Either the contribution advice or
the servers should be changed, ideally!)

I understand that one can host patches somewhere official, but I've
forgotten the details of the process.

Can anyone help?

Jonathan


On 6 April 2017 at 15:08, Chris Dennis  wrote:

> Hi Paul (et al)
>
> Like all things API there are wrinkles here when it comes to implementing.
>
> This patch isn’t final, there appears to be no existing test coverage for
> these classes beyond testing the compensating summation used in the double
> implementation, and I left off adding any until it was decided how much
> parameter validation we want (since that’s the only testing that can really
> occur here).
>
> The wrinkles relate to the issues around instances that have suffered
> overflow in count and/or sum which the existing implementation doesn’t
> defend against:
>
>  * I chose to ignore all parameters if 'count == 0’ and just leave the
> instance empty. I held off from validating min, max and count however. This
> obviously 'breaks things’ (beyond how broken they would already be) if
> count == 0 only due to overflow.
>  * I chose to fail if count is negative.
>  * I chose to enforce that max and min are logically consistent with count
> and sum, this can break the moment either one of the overflows as well (I’m
> also pretty sure that the FPU logic is going to suffer here too - there
> might be some magic I can do with a pessimistic bit of rounding on the FPU
> multiplication result).
>
> I intentionally went with the most aggressive parameter validation here to
> establish one end of what could be implemented.  There are arguments for
> this and also arguments for the other extreme (no validation at all).
> Personally I’m in favor of the current version where the creation will fail
> if the inputs are only possible through overflow (modulo fixing the FPU
> precision issues above) even though it’s at odds with approach of the
> existing implementation.
>
> Would appreciate thoughts/feedback.  Thanks.
>
> Chris
>
>
> P.S. I presume tests would be required for the parameter checking (once it
> is finalized)?
>
>

[PATCH] 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics

[Chris Dennis]

Hi Paul (et al)

Like all things API there are wrinkles here when it comes to implementing.

This patch isn’t final, there appears to be no existing test coverage for these 
classes beyond testing the compensating summation used in the double 
implementation, and I left off adding any until it was decided how much 
parameter validation we want (since that’s the only testing that can really 
occur here).

The wrinkles relate to the issues around instances that have suffered overflow 
in count and/or sum which the existing implementation doesn’t defend against:

 * I chose to ignore all parameters if 'count == 0’ and just leave the instance 
empty. I held off from validating min, max and count however. This obviously 
'breaks things’ (beyond how broken they would already be) if count == 0 only 
due to overflow.
 * I chose to fail if count is negative.
 * I chose to enforce that max and min are logically consistent with count and 
sum, this can break the moment either one of the overflows as well (I’m also 
pretty sure that the FPU logic is going to suffer here too - there might be 
some magic I can do with a pessimistic bit of rounding on the FPU 
multiplication result).

I intentionally went with the most aggressive parameter validation here to 
establish one end of what could be implemented.  There are arguments for this 
and also arguments for the other extreme (no validation at all).  Personally 
I’m in favor of the current version where the creation will fail if the inputs 
are only possible through overflow (modulo fixing the FPU precision issues 
above) even though it’s at odds with approach of the existing implementation.

Would appreciate thoughts/feedback.  Thanks.

Chris


P.S. I presume tests would be required for the parameter checking (once it is 
finalized)?

Re: 9 RFR: 8178139: Minor typo in API documentation of java.util.logging.Logger

[Daniel Fuchs]

On 06/04/2017 12:02, Lance Andersen wrote:

looks fine Daniel


Thanks Lance. Pushed.

-- daniel


On Apr 6, 2017, at 7:00 AM, Daniel Fuchs > wrote:

Hi,

Please find below a trivial change to fix
8178139: Minor typo in API documentation of java.util.logging.Logger
https://bugs.openjdk.java.net/browse/JDK-8178139

  will configured => will be configured
  (appears twice)

This is a minor typo doc fix so AFAIU RDP2 approval is not needed.

best regards,

-- daniel

diff --git
a/src/java.logging/share/classes/java/util/logging/Logger.java
b/src/java.logging/share/classes/java/util/logging/Logger.java
--- a/src/java.logging/share/classes/java/util/logging/Logger.java
+++ b/src/java.logging/share/classes/java/util/logging/Logger.java
@@ -664,7 +664,7 @@
 * a new logger is created.
 * 
 * If a new logger is created its log level will be configured
- * based on the LogManager configuration and it will configured
+ * based on the LogManager configuration and it will be configured
 * to also send logging output to its parent's Handlers.  It will
 * be registered in the LogManager global namespace.
 * 
@@ -726,7 +726,7 @@
 *
 * 
 * If a new logger is created its log level will be configured
- * based on the LogManager and it will configured to also send
logging
+ * based on the LogManager and it will be configured to also send
logging
 * output to its parent's Handlers.  It will be registered in
 * the LogManager global namespace.
 * 




Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

RE: JDK 9 RFR(s): 8150488: add note to Scanner.findAll()regardingpossible infinite streams

[Timo Kinnunen]

IMHO there should be a notice added in findAll which excludes the behavior of 
the stream after an empty match from any compatibility requirements while the 
notice remains in place. This would be to ensure that findAll and the stream it 
returns can be changed independently from the other methods which also have 
this issue. Just in case.



Sent from Mail for Windows 10

From: Stuart Marks
Sent: Wednesday, April 5, 2017 18:49
To: Xueming Shen
Cc: core-libs-dev@openjdk.java.net
Subject: Re: JDK 9 RFR(s): 8150488: add note to 
Scanner.findAll()regardingpossible infinite streams



On 4/4/17 10:48 PM, Stuart Marks wrote:
>
>
> On 4/4/17 4:10 PM, Xueming Shen wrote:
>> Personally I think the use scenario and the expected resulting behavior of
>> StreamfinaAll(ptn) should be more equivalent/similar to the use case  of
>> while (s.hasNext(p)) { s.next(p); }, or while (m.find()) { }, therefor it is
>> probably desired it can be used without worrying the possibility of getting
>> into an infinite loop.
>
> The Scanner.tokens() API, also added in Java 9, provides a stream of delimited
> tokens (strings) that's more-or-less equivalent of a hasNext()/next() loop.
>
> Just as findWithinHorizon() is a distinct use case from hasNext()/next()
> delimited tokens, findAll() provides for distinct uses from tokens().
>
>> That said, I agree it might be hard to argue to fix it in jdk9. The question
>> is if we are going to fix it in 9u sometime in the further, is it still worth
>> putting down the note in the API now. I'm fine if you believe a note will
>> help at least make the issue a known/documented usage limitation, for JDK9.

I've thought about this some more... fundamentally the behavior is odd, since 
Scanner.findWithinHorizon() is *based on* Matcher.find(). So why does 
Matcher.find() "work" and S.findWithinHorizon() produce an infinite loop?

If you look in findPatternInBuffer, upon which findWithinHorizon and findAll 
are 
based, it essentially resets the matcher every time, which cases the 
Matcher.find() special case to be bypassed. So I think this could be considered 
a bug in findPatternInBuffer, which causes misbehavior to be visible in 
findWithinHorizon and findAll.

I've filed:

JDK-8178116 scanner.findWithinHorizon doesn't advance after matching zero 
characters

This is certainly too subtle to be trying to fix right now.

I guess the question is, if we believe this is a bug that will be fixed at some 
point, do we add a note to findAll() -- and maybe even findWithinHorizon() -- 
to 
document this? Historically we haven't documented bugs like this, so maybe not.

Note also that this behavior of findWithinHorizon has been there since JDK 6, 
which is the earliest release I had conveniently at hand.

s'marks


>>
>> Yes, I think we did chat about this one at the hallway some time when either
>> you or something ran into the loop by using the method ...
>
> There are already a couple ways you can get unexpected behavior with Scanner 
> if
> you're not careful. For example, see the warnings in findWithinHorizon() and
> skip() about the possibility of buffering up the entire input. These methods 
> are
> useful, but if you're not careful with the regex you provide, things can go
> wrong. The findAll() method seems to be in the same situation. It's not clear 
> to
> me that any of these are bugs for which there's a reasonable fix.
>
> So yes, I think the note is necessary. I could also add something about
> terminating such streams with limit() or takeWhile().
>
> s'marks
>
>>
>> Thanks,
>> Sherman
>>
>>
>>>
>>> s'marks
>>>
>>> On 3/30/17 2:19 PM, Stuart Marks wrote:
 Hi Timo, Sherman,

 Thanks for looking at this.

 Sherman wrote:

> This might practically put the api itself almost useless? it might be an 
> easy
> task to spot
> whether or not it's a 0-width-match-possible regex when the regex is 
> simple,
> but it gets
> harder and harder, if not impossible when the regex gets complicated,
> especially consider
> the possible use scenario that the use site is embedded deeply inside a
> library implementation.

 Well, not "useless", but perhaps less useful than one might like. :-)

 I think this is potentially surprising behavior, which is why I at least 
 wanted
 to add the note. It's not clear to me whether we should try to fix this by
 changing Scanner though.

 Essentially, findAll() is defined in terms of findWithinHorizon(pattern, 
 0). So
 if one were to write a loop like so:

 String str;
 while ((str = scanner.findWithinHorizon(pattern, 0)) != null) {
 System.out.println(str);
 }

 then this loop would have the same problem if pattern were to match zero
 characters.

> The alternative is to "fix" it, maybe as what Matcher.find() does, if the
> previous match is
> zero-width-match (the fist==last), we step one to the next 

Re: 9 RFR: 8178139: Minor typo in API documentation of java.util.logging.Logger

[Lance Andersen]

looks fine Daniel
> On Apr 6, 2017, at 7:00 AM, Daniel Fuchs  wrote:
> 
> Hi,
> 
> Please find below a trivial change to fix
> 8178139: Minor typo in API documentation of java.util.logging.Logger
> https://bugs.openjdk.java.net/browse/JDK-8178139
> 
>   will configured => will be configured
>   (appears twice)
> 
> This is a minor typo doc fix so AFAIU RDP2 approval is not needed.
> 
> best regards,
> 
> -- daniel
> 
> diff --git a/src/java.logging/share/classes/java/util/logging/Logger.java 
> b/src/java.logging/share/classes/java/util/logging/Logger.java
> --- a/src/java.logging/share/classes/java/util/logging/Logger.java
> +++ b/src/java.logging/share/classes/java/util/logging/Logger.java
> @@ -664,7 +664,7 @@
>  * a new logger is created.
>  * 
>  * If a new logger is created its log level will be configured
> - * based on the LogManager configuration and it will configured
> + * based on the LogManager configuration and it will be configured
>  * to also send logging output to its parent's Handlers.  It will
>  * be registered in the LogManager global namespace.
>  * 
> @@ -726,7 +726,7 @@
>  *
>  * 
>  * If a new logger is created its log level will be configured
> - * based on the LogManager and it will configured to also send logging
> + * based on the LogManager and it will be configured to also send logging
>  * output to its parent's Handlers.  It will be registered in
>  * the LogManager global namespace.
>  * 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

9 RFR: 8178139: Minor typo in API documentation of java.util.logging.Logger

[Daniel Fuchs]

Hi,

Please find below a trivial change to fix
8178139: Minor typo in API documentation of java.util.logging.Logger
https://bugs.openjdk.java.net/browse/JDK-8178139

   will configured => will be configured
   (appears twice)

This is a minor typo doc fix so AFAIU RDP2 approval is not needed.

best regards,

-- daniel

diff --git 
a/src/java.logging/share/classes/java/util/logging/Logger.java 
b/src/java.logging/share/classes/java/util/logging/Logger.java

--- a/src/java.logging/share/classes/java/util/logging/Logger.java
+++ b/src/java.logging/share/classes/java/util/logging/Logger.java
@@ -664,7 +664,7 @@
  * a new logger is created.
  * 
  * If a new logger is created its log level will be configured
- * based on the LogManager configuration and it will configured
+ * based on the LogManager configuration and it will be configured
  * to also send logging output to its parent's Handlers.  It will
  * be registered in the LogManager global namespace.
  * 
@@ -726,7 +726,7 @@
  *
  * 
  * If a new logger is created its log level will be configured
- * based on the LogManager and it will configured to also send logging
+ * based on the LogManager and it will be configured to also send 
logging

  * output to its parent's Handlers.  It will be registered in
  * the LogManager global namespace.
  * 

Re: JDK 10 RFR of 8176894: Provide specialized implementation for default methods putIfAbsent, computeIfAbsent, computeIfPresent, compute in TreeMap

[Claes Redestad]

Hi Sergey,

this looks good to me*, but I can't help wonder if the modCount checking 
is something that should be done separately as a bug fix (with a higher 
priority) and be backported to 8 and 9? Alternatively re-categorize this 
fix as such.


Thanks!

/Claes

* I wouldn't mind seeing the cleanup Martin suggested, i.e., write the 
remapValue as:


private V remapValue(Entry t, K key, BiFunctionsuper V, ? extends V> remappingFunction) {

V newValue = remappingFunction.apply(key, t.value);
if (newValue == null) {
deleteEntry(t);
} else {
// replace old mapping
t.value = newValue;
}
return newValue;
 }

On 2017-03-28 21:22, Sergey Kuksenko wrote:

Friendly reminder.

Have you have chance to review the latest version?


On 03/17/2017 12:45 PM, Sergey Kuksenko wrote:

Hi, All.

I realized (thanks Tagir V.) that if we don't check modCount after 
calling mapping function we may get corrupted tree structure.

new webrev for review:
http://cr.openjdk.java.net/~skuksenko/corelibs/utils/8176894/webrev.01/

On 03/17/2017 11:29 AM, Martin Buchholz wrote:
Thanks!  This looks pretty good.  I have a similar effort in 
progress to improve bulk collection operations, most of which made 
it into jdk9.


---

Please use standard java.util whitespace, as Aleksey suggested.

---

Below (and in compute) I wpuld simply
return newValue;
saving a line of code and making it clearer that we are returning 
the result of the remappingFunction


 676 private V remapValue(Entry t, K key, BiFunctionsuper K, ? super V, ? extends V> remappingFunction) {

 677 V newValue = remappingFunction.apply(key, t.value);
 678 if (newValue == null) {
 679 deleteEntry(t);
 680 return null;
 681 } else {
 682 // replace old mapping
 683 t.value = newValue;
 684 return newValue;
 685 }
 686 }
---

This code is surely tested but testing could also surely be 
improved.  That's probably not your job though (it may be mine!)


I would probably try hand-injecting some bugs into a copy of the 
code and seeing if our jtreg tests catch it, to increase coverage 
confidence.



On Thu, Mar 16, 2017 at 12:04 PM, Sergey Kuksenko 
> wrote:


Hi All,

Please, review:
https://bugs.openjdk.java.net/browse/JDK-8176894

http://cr.openjdk.java.net/~skuksenko/corelibs/utils/8176894/webrev.00/
 



The issue was created for JDK10 in order to don't disturb JDK9
before launch.

-- Best regards,
Sergey Kuksenko