ICU-22616 fix value returned by Calendar::getTimeInMillis() after call to Calendar::set() during ambiguous time

[cjchapman]

Checklist

• Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22616
• Required: The PR title must be prefixed with a JIRA Issue number.
• Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
• Required: Each commit message must be prefixed with a JIRA Issue number.
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable

[CLAassistant]

All committers have signed the CLA.

[poulsbo]

I'm bringing over my thoughts from ICU-22616. I think Christopher Chapman did nice work on the PR, but I believe there are some risks in making this change, and that it may be unnecessary if a small change to user code can accomplish the same thing.

1. I had the impression that setRepeatedWallTimeOption was meant to be a user-called function. That is, the framework would not call it, allowing users to control this behavior. If so, then calling it within the framework like this PR proposes might break use cases where the user wants to explicitly set it. Yoshito Umaoka offers comments here and here.

2. Setting the above concern aside, a good set of tests for such a change would cover, in both C and Java, (a) a point in time in the middle of standard time, (b) a point in time in the middle of daylight savings time, and (c) the transition hour. I only see tests for (c) in this PR.

Could the same outcome be achieved by modifying the calling code to add a call to setRepeatedWallTimeOption after setting the ERA field? If so, then a safe decision is to not change the ICU framework code (not accept this PR), and optionally add to the ICU documentation so setRepeatedWallTimeOption is referenced in the doc for API dealing with fields and millis.

If the consensus among ICU maintainers (I am not an active maintainer) is to accept this PR, then I strongly suggest narrowing the effect of the change to cases where the user has not set the local time offset. That is, the new logic should be enclosed in an if-block similar to this:

if(fStamp[ZONE_OFFSET] == kInternallySet && fStamp[DST_OFFSET] == kInternallySet) {
  // new logic
}

And tests should be added to confirm that setRepeatedWallTimeOption is not called for user-set local time offset.

Copying @yumaoka and @markusicu for visibilty.

[cjchapman]

We discussed this in the ICU-TC meeting earlier today, Marcus posted a summary of the discussion here:
https://unicode-org.atlassian.net/browse/ICU-22616?focusedCommentId=173137

Per the discussion, I'm going to hold off on this PR until we've had a chance to think about the issue some more.

@poulsbo & @yumaoka I appreciate the guidance you given me on this. I'll be happy to make the changes and add the test cases Alan recommended above should we decide to go this route.

[richgillam]

Consensus in ICU-TC is not to change the code, but to update the documentation, probably in the class overview doc for Calendar.

[richgillam]

Mark suggests:

somewhere below: *

Ambiguous Wall Clock Time. When time offset from UTC has

[FrankYFTang]

Mark suggests:

somewhere below: *

Ambiguous Wall Clock Time. When time offset from UTC has

I think your comment is incomplete .

[markusicu]

TC discussion: Better for callers to be explicit about ambiguous-time resolution.