[m-rev.] diff/for post-commit review: use explicit streams in some library modules

Julien Fischer jfischer at opturion.com
Sun Oct 30 12:39:53 AEDT 2016



Hi Zoltan,

On Sun, 30 Oct 2016, Zoltan Somogyi wrote:

> Most of the diff is boring and safe. The only part that could do
> with a review is the second chunk in the diff of io.m: was there
> some non-bug reason for the old behavior of the code that reads
> bitmaps?

Not that I'm ware of; it looks like a bug.


> diff --git a/library/set_ctree234.m b/library/set_ctree234.m
> index 8600215..93c3af3 100644
> --- a/library/set_ctree234.m
> +++ b/library/set_ctree234.m
> @@ -700,7 +700,9 @@ do_from_sorted_list(Len, !List, Level0, AllThrees0, Tree) :-
>              ),
>
>              trace [io(!IO), compile_time(flag("from_sorted_list"))] (
> -                io.format("splitting %d into three: %d, %d, %d\n",
> +                io.output_stream(SplitStream, !IO),
> +                io.format(SplitStream,
> +                    "splitting %d into three: %d, %d, %d\n",
>                      [i(Len), i(SubLen1), i(SubLen2), i(SubLen3)], !IO)
>              ),
> 
> @@ -721,9 +723,10 @@ do_from_sorted_list(Len, !List, Level0, AllThrees0, Tree) :-
>              do_from_sorted_list(SubLen3, !List, Level, AllThrees, SubTree3),
>              Tree = three(E1, E2, SubTree1, SubTree2, SubTree3),
>              trace [io(!IO), compile_time(flag("from_sorted_list"))] (
> -                io.format("tree for %d\n", [i(Len)], !IO),
> -                io.write(Tree, !IO),
> -                io.nl(!IO)
> +                io.output_stream(TreeStream, !IO),
> +                io.format(TreeStream, "tree for %d\n", [i(Len)], !IO),
> +                io.write(TreeStream, Tree, !IO),
> +                io.nl(TreeStream, !IO)

You could use io.write_line there and in other spots.

...

> diff --git a/library/term_io.m b/library/term_io.m
> index 98b394f..8139f74 100644
> --- a/library/term_io.m
> +++ b/library/term_io.m
> @@ -36,58 +36,80 @@
>
>  :- type read_term == read_term(generic).
> 
> -    % read_term(Result, !IO):
> +    % Read a term from the current input stream or from the given input stream.
> +    %
> +    % Similar to NU-Prolog read_term/2, except that resulting term
> +    % is in the ground representation.

A separate matter: I don't think comparisons to NU-Prolog are going to be
that meaningful to most of our users.

The diff looks fine.

Julien.


More information about the reviews mailing list