[m-rev.] for review: rewrite rest of options_file.m

[Julien Fischer]

Hi Zoltan,

On Mon, 8 Jun 2020, Zoltan Somogyi wrote:

> For review by anyone.
>
> Before commit, I intend to reorder the contents of options_file.m,
> putting related predicates next to each other, but this diff leaves
> predicates in their current places to make review easier.
>
> Does anyone know whether the functionality of options_file.m has any
> test cases for it, apart from Mercury.config and Mercury.options files?

We don't have any additional tests of that functionality.

> I couldn't find any. I do think there should be some, but I am not sure
> whether these should go in e.g. tests/hard_coded, or in a new subdirectory
> of tests dedicated to tests of option file handling.

It should be a new subdirectory; there's enough complication in
hard_coded as it is.

(Incidentally, running the test suite directly, by doing "mmake
runtests" in the top-level tests directory, no longer works due to your
changes back in April.  Is that intentional?)

> (In those tests, the code in the module(s) being compiled almost
> doesn't matter, which is a pretty clear point of difference from the
> contents of all the other test directories).

Maybe the way we should do option file testing is to have an additional
output auxiliary output mode for the compiler that dumps the final
variable - value mapping from all the options files processed in some
canonical form?

> Any preferences? And does anyone have options files in their own
> projects that go beyond the usual Mercury.{config,options} files in
> their complexity that I could use as (parts of) test cases?

I can provide you with some from G12; however I don't think they're
intrinsically more complex than anything you will find in the Mercury
system (so far as options_file.m is concerned).

> Do not use exceptions in options_file.m.
> 
> compiler/options_file.m:
>     This diff rewrites options_file.m in a straightforward, direct style that
>     returns indications of errors as error_specs rather than as exceptions.
>     A recent diff started on this task; this diff finishes it.
>
>     The new approach has several advantages.
>
>     - The control flow is much simpler, and therefore more understandable.
>       Correctness arguments for propositions such as "this code closes
>       all the file streams that it opens" are now much simpler to make.
>
>     - We now report errors using error_specs, which contain context
>       information, while previously, each error was described only
>       by a string, without context info.
>
>     - Once we detect and report one error, we can continue to read the
>       rest of the input. This allows a single compiler invocation to find
>       and report several errors, not just the first.
>
>     - Since we now return the gathered set of error_specs instead of printing
>       them, the predicates of this file don't have to take globals structures
>       as arguments, which allows our callers to avoid constructing those
>       structures.
>
>     - Deep profiling, which cannot handle exceptions, now works on
>       the code of this module.
>
>     change over to using trace goals for debugging prints, since continuing
>     to use debug_make_msg would require a globals structure.

Capitalize change.

>     Add an XXX on a likely bug.

...

One general commment: I think all of the call to be io.read_char should be replaced
by calls to io.read_char_unboxed.

The diff looks fine.

Julien.