RFR: 8367901: Calendar.roll(hour, 24) returns wrong result [v2]

Naoto Sato naoto at openjdk.org
Thu Sep 18 22:22:14 UTC 2025


On Thu, 18 Sep 2025 20:41:58 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR which addresses an edge case for `GregorianCalendar.roll(int, int)` when the rolled amount would cause the hour to remain the same as before the call. After this change, the expected hour is returned. That is, rolling a full cycle for HOUR (12 hours) and HOUR_OF_DAY (24 hours) should keep the hour the same as before the call.
>> 
>> For example, a calendar with HOUR_OF_DAY == 15,
>> 
>> 
>> cal.roll(Calendar.HOUR_OF_DAY, 23);
>> cal.get(Calendar.HOUR_OF_DAY); // returns 14
>> 
>> 
>> cal.roll(Calendar.HOUR_OF_DAY, 24);
>> // Incorrectly returns 16. A full cycle is expected to return the starting hour (15)
>> cal.get(Calendar.HOUR_OF_DAY); 
>> 
>> 
>> cal.roll(Calendar.HOUR_OF_DAY, 25);
>> cal.get(Calendar.HOUR_OF_DAY); // returns 16
>
> Justin Lu has updated the pull request incrementally with two additional commits since the last revision:
> 
>  -  + - to - in comment
>  - DST test cases + compute expected value as method + improve err msg

LGTM

test/jdk/java/util/Calendar/RollHoursTest.java line 73:

> 71:         calendars.get(0).set(2005, 8, 20, 12, 10, 25);
> 72: 
> 73:         // Transition to daylight savings time (CST/CDT) ---

Nit: I think it is usually called "daylight saving time"

-------------

Marked as reviewed by naoto (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27355#pullrequestreview-3242247888
PR Review Comment: https://git.openjdk.org/jdk/pull/27355#discussion_r2361302216


More information about the core-libs-dev mailing list