[m-rev.] for review: explicit streams in the browser directory

Julien Fischer jfischer at opturion.com
Fri Mar 5 20:02:25 AEDT 2021


Hi Zoltan,

On Fri, 5 Mar 2021, Zoltan Somogyi wrote:

> For review by anyone. Besides the diff to the browser,
> there are also two small diffs for your info that should not
> need review.

...

> Use explicit streams in browser/*.m.
> 
> browser/browse.m:
> browser/browser_info.m:
> browser/collect_lib.m:
> browser/declarative_debugger.m:
> browser/declarative_oracle.m:
> browser/declarative_user.m:
> browser/diff.m:
> browser/help.m:
> browser/interactive_query.m:
> browser/parse.m:
> browser/util.m:

...


> diff --git a/browser/interactive_query.m b/browser/interactive_query.m
> index 4be26d747..7688b1bd5 100644
> --- a/browser/interactive_query.m
> +++ b/browser/interactive_query.m

...


>      % Invoke the Mercury compile to compile the file to a shared object.
>      %
> -:- pred compile_file(options_string::in, bool::out, io::di, io::uo) is det.
> +:- pred compile_file(io.text_output_stream::in, options_string::in, bool::out,
> +    io::di, io::uo) is det.
> 
> -compile_file(Options, Succeeded, !IO) :-
> +compile_file(OutputStream, Options, Succeeded, !IO) :-
>      % We use the following options:
>      %   --grade
>      %       make sure the grade of libmdb_query.so matches the
> @@ -633,24 +651,24 @@ compile_file(Options, Succeeded, !IO) :-
>          " --compile-to-shared-lib ",
>          query_module_name],
>          Command),
> -    invoke_system_command(Command, Succeeded, !IO).
> +    invoke_system_command(OutputStream, Command, Succeeded, !IO).
>
>  :- pred cleanup_query(string::in, io::di, io::uo) is det.
> 
> -cleanup_query(_Options) -->
> -    cleanup_file("", ".m"),
> -    cleanup_file("", ".mh"),
> -    cleanup_file("", ".d"),
> -    cleanup_file("Mercury/ds/", ".d"),
> -    cleanup_file("", ".c"),
> -    cleanup_file("Mercury/cs/", ".c"),
> -    cleanup_file("", ".c_date"),
> -    cleanup_file("Mercury/c_dates/", ".c_date"),
> -    cleanup_file("", ".o"),
> -    cleanup_file("Mercury/os/", ".o"),
> -    cleanup_file("", ".pic_o"),
> -    cleanup_file("Mercury/os/", ".pic_o"),
> -    cleanup_file("lib", ".so").
> +cleanup_query(_Options, !IO) :-
> +    cleanup_file("", ".m", !IO),
> +    cleanup_file("", ".mh", !IO),
> +    cleanup_file("", ".d", !IO),
> +    cleanup_file("Mercury/ds/", ".d", !IO),
> +    cleanup_file("", ".c", !IO),
> +    cleanup_file("Mercury/cs/", ".c", !IO),
> +    cleanup_file("", ".c_date", !IO),
> +    cleanup_file("Mercury/c_dates/", ".c_date", !IO),
> +    cleanup_file("", ".o", !IO),
> +    cleanup_file("Mercury/os/", ".o", !IO),
> +    cleanup_file("", ".pic_o", !IO),
> +    cleanup_file("Mercury/os/", ".pic_o", !IO),
> +    cleanup_file("lib", ".so", !IO).

There's a couple of existing problems there.  The object file extension is not
always ".o" (although at the moment, it will be so on any system that actually
supports interactive queries.)  The shared object extension is *not* always
".so", on macOS (which does support interactive queries) it will be ".dylib".

I would add an XXX about the first and change the last line to:

     cleanup_file("lib", shlib_extension, !IO).

> -write_binding(Bindings, Output, !IO) :-
> +write_binding(OutputStream, Bindings, Output, !IO) :-
>      map.lookup(Bindings, Output, Univ),
> -    io.write_string(Output, !IO),
> -    io.write_string(" = ", !IO),
> -    io.write_cc(univ_value(Univ), !IO),
> -    io.write_string(", ", !IO).
> +    io.write_string(OutputStream, Output, !IO),
> +    io.write_string(OutputStream, " = ", !IO),
> +    io.write_cc(OutputStream, univ_value(Univ), !IO),
> +    io.write_string(OutputStream, ", ", !IO).
> 
> -:- pred report_exception(T::in, io::di, io::uo) is cc_multi.
> +:- pred report_exception(io.text_output_stream::in, T::in, io::di, io::uo)
> +    is cc_multi.
> 
> -report_exception(Excp, !IO) :-
> -    io.write_string("*** caught exception: ", !IO),
> -    io.write_cc(Excp, !IO),
> -    io.nl(!IO).
> +report_exception(OutputStream, Excp, !IO) :-
> +    io.write_string(OutputStream, "*** caught exception: ", !IO),
> +    io.write_cc(OutputStream, Excp, !IO),
> +    io.nl(OutputStream, !IO).

Hmmm, we should definitely add a version of write_line_cc that takes an
output_stream argument.

The rest looks fine.

Julien.


More information about the reviews mailing list