[m-rev.] for review: add 64-bit versions of seek and offset on binary streams

Zoltan Somogyi zoltan.somogyi at runbox.com
Tue Oct 8 20:51:47 AEDT 2019



On Tue, 8 Oct 2019 20:43:11 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
> > You have not pushed that commit yet.
> 
> No, I was waiting for it to be reviewed ;-)

I was waiting for someone who knows Windows better ... :-(

> >
> >>       % Returns the offset (in bytes) into the specified binary output stream.
> >>       %
> >>   :- pred binary_output_stream_offset(io.binary_output_stream::in, int::out,
> >>       io::di, io::uo) is det.
> >>
> >> +    % As above, but the offset is always a 64-bit value.
> >> +    %
> >> +:- pred binary_output_stream_offset64(io.binary_output_stream::in, int64::out,
> >> +    io::di, io::uo) is det.
> >
> > Do you intend for the final predicates names to have the 64 suffix?
> 
> No, the final versions will use the unsuffixed names but with int64
> offset arguments.  (It will take us a few releases to get there though.)

Ok, good. That is my preference as well.

> > I would have thought that we could have three versions now:
> > plain, _32 suffix, and _64 suffix. For now, the plain version would redirect
> > to the _32 version, with an obsolete pragma in favor of the _32 version,
> > but would then be made identical to the_64 version, with a pragma obsolete on the
> > _64 version in favor of the plain version.
> 
> The problem is that the existing version is not necessarily 32-bit
> already -- what it is varies depending on what size a Mercury int is and
> whether we are on Windows or not (i.e. it is already 64-bit on many
> platforms but Mercury programmers cannot rely on that).

Right.

> The right thing to do is transition users to a version that always uses
> 64-bit offsets.  All the underlying platforms provide that anyway.

My email proposed such as plan. It is sort-of still valid if you replace
the _32 suffix in names with something else that suggests pot-luck
with platform int sizes.

Zoltan.


More information about the reviews mailing list