Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Span: add saturating arithmetic #10

Open
BurntSushi opened this issue Jul 6, 2024 · 0 comments
Open

Span: add saturating arithmetic #10

BurntSushi opened this issue Jul 6, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@BurntSushi
Copy link
Owner

All of the datetime types have both checked and saturating arithmetic defined for them. Saturating arithmetic is nice because it avoids needing to deal with error conditions while returning a "reasonable" result that is probably obviously wrong in the rare cases it occurs.

Saturating arithmetic for the datetime types is defined in terms of checked arithmetic. For exmaple, for Zoned:

    pub fn saturating_add(&self, span: Span) -> Zoned {
        self.checked_add(span).unwrap_or_else(|_| {
            let ts = if span.is_negative() {
                Timestamp::MIN
            } else {
                Timestamp::MAX
            };
            ts.to_zoned(self.time_zone().clone())
        })
    }

This is in general a good strategy for datetime types because there are singular minimum and maximum values for datetimes. That is, there are only two ways for them to overflow: go under the minimum or over the maximum.

But for a Span, while there is technically a minimal and maximal Span that can be defined, overflow can occur as a result of arithmetic on individual fields. So for example, adding 19_998.years() + 1.year() would overflow because it exceeds the year limit, but it doesn't exceed the maximal logical duration representable by a span.

So if we do define saturating arithmetic on a span, I do wonder how exactly this would work. Right now, this is the implementation of checked arithmetic:

    #[inline]
    fn checked_add(self, span: Span) -> Result<Span, Error> {
        let (span1, span2) = (span, self.span);
        let unit = span1.largest_unit().max(span2.largest_unit());
        let start = match self.relative {
            Some(r) => {
                if !r.is_variable(unit) {
                    return span1.checked_add_invariant(unit, &span2);
                }
                r.to_relative()?
            }
            None => {
                if unit.is_definitively_variable() {
                    return Err(err!(
                        "using largest unit (which is '{unit}') in given span \
                         requires that a relative reference time be given, \
                         but none was provided",
                        unit = unit.singular(),
                    ));
                }
                return span1.checked_add_invariant(unit, &span2);
            }
        };
        let mid = start.checked_add(span1)?;
        let end = mid.checked_add(span2)?;
        start.until(unit, &end)
    }

Notice how the arithmetic is actually arithmetic on datetime and span types. I do wonder if we can make this saturating by copying it and using saturating arithmetic for the datetime + span operations. But I haven't fully thought through its semantics.

@BurntSushi BurntSushi added the enhancement New feature or request label Jul 6, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant