Looks good.
Give it a day to see if anyone else has comments.
Thanks, Roger
On 3/2/20 1:35 PM, naoto.s...@oracle.com wrote:
Hi Roger, thanks for the review.
On 3/2/20 8:44 AM, Roger Riggs wrote:
Hi Naoto,
look ok.
ZoneRules.java: 488, 644, 761, 791
I'd suggest calling isFixedOffset() instead of repeating the code:
standardTransitions.length == 0 && savingsInstantTransitions.length == 0
Modified as suggested:
http://cr.openjdk.java.net/~naoto/8239836/webrev.01/
It should be inlined in cases where the performance matters.
None of those locations is invoked during ZoneRules object
instantiation. So I believe it is OK to replace them with
isFixedOffset().
Naoto
Thanks, Roger
On 2/27/20 3:41 PM, naoto.s...@oracle.com wrote:
Hello,
Please review the fix to the following issue:
https://bugs.openjdk.java.net/browse/JDK-8239836
The proposed changeset is located at:
https://cr.openjdk.java.net/~naoto/8239836/webrev.00/
It turned out that 'transitionList' is not necessarily a superset of
'standardOffsetTransitionList', as in some occasions where a
standard offset change and a savings change happen at the same time
(canceling each other), resulting in no wall time transition. This
means that the "rules" in the sample code is a valid ZoneRules
instance.
However, there is an assumption in ZoneRules where no (wall time)
transition means the fixed offset zone. With that assumption, the
example rule is considered a fixed offset zone which is not correct.
So the fix proposed here is not to throw an IAE but to recognize the
rule as a valid, non-fixed offset ZoneRules instance.
Naoto
