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.sato@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