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

Julien Fischer jfischer at opturion.com
Wed Apr 22 14:43:35 AEST 2026


On Wed, 22 Apr 2026 at 13:30, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
>
> 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.

Done.

> > 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.

I have replaced that with:

    Currently, the clock_t type in the time module of the standard library is an
    equivalence type for int.  On platforms where Mercury's int type is 32 bits,
    this overflows after just minutes of CPU time.

Similarly in the NEWS file.

> > 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.

I have added such a comment.

> > @@ -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.

I will take a look.  That behaviour is inherited from C.
(In the longer term, I think we will probably replace this entire module anyway;
for now, I am just going to fix some of its more egregious problems.)

> > +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

Failing loudly in this case will halt your program. For some programs,
it may not matter too much if clock_t values overflow (e.g. logging
output).  The intention with clock_t_to_int is that you can replicate
the existing (broken) behaviour.  The correct fix is to update to your
code to use the 64-bit version.  (Hopefully, the added obsolete
pragma will encourage that.)

Julien.


More information about the reviews mailing list