Decide on overflow behavior for calendar arithmetic

[sffc]

There are various places in the calendar code where overflow can occur for very large or small dates. For instance,

    pub(crate) fn fixed_from_iso(date: IsoDateInner) -> i32 {
        // Calculate days per year
        let mut fixed: i32 = EPOCH - 1 + 365 * (date.0.year - 1);
        // Adjust for leap year logic
        fixed += ((date.0.year - 1) / 4) - ((date.0.year - 1) / 100) + ((date.0.year - 1) / 400);
        // Days of current year
        fixed += (367 * (date.0.month as i32) - 362) / 12;
        // Leap year adjustment for the current year
        fixed += if date.0.month <= 2 {
            0
        } else if Self::is_leap_year(date.0.year) {
            -1
        } else {
            -2
        };
        // Days passed in current month
        fixed + (date.0.day as i32)
    }

It is fairly easy to trigger overflow here because date.0.year is an i32, so if it is larger than i32::MAX / 365, it will overflow.

We have a few options:

1. Ignore it. Rely on the Rust integer overflow debug assertion to catch issues during testing.
2. Make operations that could cause overflow to occur be fallible.
3. Use saturating operations, such that we return things like i32::MAX instead of wrapping around.
4. Set limits on the minimum and maximum representable dates, such as those specified in ECMA-262, and choose appropriately-sized data types from all relevant functions in order to limit the number of fallible functions (constructors and arithmetic only).

For the purposes of ICU4X 1.0, we can either:

1. Keep the status quo, with the possibility to migrate to option 3 without breaking API stability, and consider a fully-thought-through approach in the 2.0 timeframe
2. Pre-emptively make some subset of the functions be fallible in order to adopt option 2 or 4

Thoughts?

• @Manishearth
• @pandusonu2
• @samchen61661

[Manishearth]

In general I think the only actual public-facing overflow is the year thing, yes? I'm okay with declaring it only valid within a certain range, and for now documented behavior as being undefined outside that range (potentially moving to a clamping solution in the future)

[sffc]

Discussion:

• @Manishearth - These are edge cases. I think saturating is fine.
• @nordzilla - I like saturating things.
• @Manishearth - With wraparound, you know something went wrong; you might not realize it with saturating. But I think saturating makes more sense. Overflow feels like a bug; saturating feels like you used it wrong, which is better.
• @sffc - I think we should either have fallible APIs or use saturating operations. In both cases, users will be able to detect the overflow: it's just whether it's explicit (unwrap a result) or implicit (check for i32::MAX in the return value). I thought people would prefer Result.
• @Manishearth - People prefer Result over panic. But Result seems like overkill as well, so a GIGO-like behavior like saturating makes sense.
• @Manishearth - I'm also okay documenting the range of valid years and saying that you might get GIGO behavior if you go outside of that range.
• @sffc - We could have a function that returns a boolean for whether the year is "safe" or not.

Conclusion: Use saturating operations. This does not affect API so it can be done in 1.x

[sffc]

Action for whoever takes this ticket:

1. Audit the datetime and calendar code to find integer overflows
2. Add tests for those integer overflows
3. Fix them by using saturating operations
4. Consider adding APIs to check if things are in range

[sffc]

There was an unconference session about integer overflow at RustConf, which largely validated the idea that saturating operations are a reasonable approach here.