[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