[m-rev.] for review: make clock_t abstract and widen its representation

Zoltan Somogyi zoltan.somogyi at runbox.com
Wed Apr 22 13:30:14 AEST 2026



On Wed, 22 Apr 2026 13:10:21 +1000, Julien Fischer <jfischer at opturion.com> wrote:

> For review by anyone.
> 
> Question for reviewers: should the newly added clock_t_to_int/0 function be
> marked as obsolete from the outset?

Yes.

> This overflows on 32-bit C backends, and on
> the Java and C# backends it overflowed within minutes of CPU time because
> the foreign_proc implementations of target_clock/1 and target_times/5
> were narrowing their (already 64-bit) underlying values down to int.

This double-barreled sentence, which is repeated in NEWS.md, implies that
there are two separate problems here, but there is only one.
 
> library/time.m:
>     Make clock_t/0 abstract.
> 
>     Use int64 as the underlying representation of clock_t values, not int.

Have you considered using uint64 instead?

LATER: I see that this would complicate the C#/Java implementations.
I would document this fact next to the type's definition.
 
> @@ -31,7 +31,7 @@
>      % returned by `clock' or `times'. See the comments on these
>      % predicates below.
>      %
> -:- type clock_t == int.
> +:- type clock_t.

Having the same type represent values measured in two different units
seems to me to be itself a bug. You may want to tackle that next.
 
> +clock_t_to_int(clock_t(Int64)) = int64.cast_to_int(Int64).

Why not int64.det_to_int? Failing loudly is better in my opinion
than quietly returning garbage.

The diff seems otherwise fine.

Zoltan.





More information about the reviews mailing list