[m-rev.] for post-commit review: organize array.m, bt_array.m and pprint.m

Julien Fischer jfischer at opturion.com
Thu Jun 9 10:47:02 AEST 2022


Hi Zoltan,

On Wed, 8 Jun 2022, Zoltan Somogyi wrote:

> The diff mostly just moves code around, so it is not worth reviewing.
> The parts worth looking at are
>
> - the groups into which the exported predicates been moved, and
> - the comments on those exported predicates.
>
> This diff also raises a question: should we make the informal
> "submodule" for random-access lists in bt_array.m an actual
> documented module of the standard library, and if so, under
> what name? I vote yes, but am agnostic about the name.
> random_access_list would be more descriptive, but ra_list
> is shorter.

On the name: I think ra_list is fine.  (After all, we already have
kv_list, rtree, rbtree, etc. and the longer name is unwieldly.)

I'm not convinced that ra_lists need to be a documented part
of the standard library, but don't really have any objection if
others feel they should be.

(Actually, I think the bt_array module should be dropped from the
standard library entirely, since it doesn't really provide anything
that the array or version_array modules don't also provide.)


> Organize array.m, bt_array.m and pprint.m.

...

> diff --git a/library/array.m b/library/array.m
> index a5acca563..95d6ec77a 100644
> --- a/library/array.m
> +++ b/library/array.m

...

> +%---------------------------------------------------------------------------%
> +%
> +% Sorting arrays.
> +%
> +
> +    % sort(Array) returns a version of Array sorted into ascending order.
> +    %
> +    % This sort is not stable. That is, elements that compare/3 decides are
> +    % equal will appear together in the sorted array, but not necessarily
> +    % in the same order in which they occurred in the input array. This is
> +    % primarily only an issue with types with user-defined equivalence for
> +    % which `equivalent' objects are otherwise distinguishable.
> +    %
> +:- func sort(array(T)::array_di) = (array(T)::array_uo) is det.
> +
> +    % array.sort was previously buggy. This symbol provides a way to ensure
> +    % that you are using the fixed version.
> +    %
> +:- pred array.sort_fix_2014 is det.
>

I think we can delete sort_fix_2014 now.

The diff looks fine.

Julien.


More information about the reviews mailing list