On Fri, 15 Sep 2023 18:54:19 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review this PR which is a continuation of >> [JDK-6453901](https://bugs.openjdk.org/browse/JDK-6453901) to remove unused >> code from the _sun.util.Calendar_ classes. >> >> `forceStandardTime` is always false. >> >> In addition, `locale` is never by used by _CalendarDate_ or any inheritors >> and can be removed. >> >> As a result, _ImmutableGregorianDate_ no longer needs to override the >> _setLocale_ method and throw UnsupportedOperationException. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify implementation after removal of if else block src/java.base/share/classes/sun/util/calendar/AbstractCalendar.java line 176: > 174: // as 1:30am DT/0:30am ST (before transition) > 175: if (zi instanceof ZoneInfo zInfo) { > 176: // Offset value adjusts accordingly depending on DST > status of date Historically, this `if else` has not been touched since the introduction of the class. The original code has a structure that one can presume follows the logic, if `isStandardTime()`, get a standard offset, otherwise get a day light saving offset. This is not the case. The code within the `else` statement is able to retrieve the correct offset if the date is in standard **or** in day light saving time (not just a day light saving offset, as the original code would imply). Consider the following example, // Where ms is calculated from the date: LA time zone at 3-13-2016 at 4 AM (daylight saving) zoneOffset = zInfo.getOffsetsByWall(ms, new int[2]); // returns the adjusted offset, -25200000 (7 hours) // Where ms is calculated from the date: LA time zone at 1-13-2016 at 4 AM (standard) zoneOffset = zInfo.getOffsetsByWall(ms, new int[2]); // returns the standard offset, -28800000 (8 hours) Removing this code is not only safe because `isStandardTime()` is always `false` - tiers 1-3 clean - breakpoint within the original `if` never breaks execution for JDK Calendar tests But we can also feel that the change is not suspicious since the code within the `else` block can produce a standard or daylight offset. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15726#discussion_r1327678366
