[m-rev.] for review: overflow checks for string conversions in the calendar module

[Zoltan Somogyi]

On Sat, 15 Aug 2026 01:36:07 +1000, Julien Fischer <jfischer at opturion.com> wrote:

> +    % This predicate fails if
> +    % - the string does not conform to the above format;
> +    % - any date or time component is outside its valid range;
> +    % - the absolute value of the year cannot be represented by Mercury's
> +    %   int type.
>      %
>  :- pred date_time_from_string(string::in, date_time::out) is semidet.

Add "or" before the third item.

> @@ -460,8 +463,11 @@
>      % This fraction component cannot include more than six digits, since
>      % the maximum resolution of a duration is a microsecond.
>      %
> -    % Fail if the string does not conform to the above format, or if the
> -    % fractional part of the seconds component has more than six digits.
> +    % This predicates fails if
> +    % - the string does not conform to the above format;
> +    % - the fractional part of the seconds component has more than six digits;
> +    % - any of the duration components cannot be represented by Mercury's
> +    %   int type.
>      %
>      % For example, the duration 1 year, 18 months, 100 days, 10 hours, 15
>      % minutes, 90 seconds and 300 microseconds can be written as:

Same here.

> +    % See the comment for string.do_base_string_to_positive_int_loop/8
> +    % for an explanation of how we check for overflow here.
> +    CutOff = max_int `unchecked_quotient` 10,
> +    CutLimit = max_int `unchecked_rem` 10,
> +    read_int_and_return_num_digits_loop(CutOff, CutLimit, 0, Int,
> +        0, NumDigits, !Chars).

I would use the naming scheme I proposed in my previous review.

I see that the diff tests for the third failure condition of both
modified exported predicates, but where are the tests for the
second conditions, for component valid range and six digits
respectively? Were they already there? If so, reword the top of
the commit message.

> +"23058430092136939520-01-01 00:00:00": bad overflow check
> +PASS: ERROR DETECTED
> diff --git a/tests/hard_coded/calendar_date_time_conv.m
> b/tests/hard_coded/calendar_date_time_conv.m
> index 994f5d788..a180d333f 100644
> --- a/tests/hard_coded/calendar_date_time_conv.m
> +++ b/tests/hard_coded/calendar_date_time_conv.m
> @@ -181,7 +181,11 @@ valid_date_times = [
>      dt_conv_test("all maximum", "2024-12-31 23:59:60.999999"),
>      dt_conv_test("Unix epoch", "1970-01-01 00:00:00"),
>      dt_conv_test("first day of the Gregorian calendar",
> -        "1582-10-15 00:00:00")
> +        "1582-10-15 00:00:00"),
> +
> +    % Valid in all grades.
> +    dt_conv_test("year on 32-bit integer boundary",
> +        "2147483647-01-01 00:00:00")
>  ].
> 
>      % These cannot be round-tripped through date_time_to_string/1.
> @@ -294,7 +298,15 @@ invalid_date_times = [
>      dt_conv_test("missing day", "2024-01- 00:00:00"),
>      dt_conv_test("missing hour", "2024-01-01 :00:00"),
>      dt_conv_test("missing minute", "2024-01-01 00::00"),
> -    dt_conv_test("trailing colon with no second digits", "2024-01-01 00:00:")
> +    dt_conv_test("trailing colon with no second digits", "2024-01-01 00:00:"),
> +
> +    % One past max_int for 64-bit integers.
> +    dt_conv_test("integer overflow in year component",
> +        "9223372036854775808-01-01 00:00:00"),
> +
> +    % A test case intended to defeat incorrect overflow checks.
> +    dt_conv_test("bad overflow check",
> +        "23058430092136939520-01-01 00:00:00")
>  ].

I don't know what kind of incorrectness you are checking for here,
because I don't know what that huge integer is in relation to max_int
for 64 bits.

> +"P9223372036854775808M":            integer overflow in months component
> +PASS: ERROR DETECTED
> +
> +"P9223372036854775808D":            integer overflow in days component
> +PASS: ERROR DETECTED
> +
> +"PT9223372036854775808H":           integer overflow in hours component
> +PASS: ERROR DETECTED
> +
> +"PT9223372036854775808M":           integer overflow in minutes component
> +PASS: ERROR DETECTED
> +
> +"PT9223372036854775808S":           integer overflow in seconds component
> +PASS: ERROR DETECTED

Wouldn't even 13 be an overflow in the months, 32 in the days component,
etc? Are those tests missing?

Zoltan.