[m-rev.] for review: insts in mode error messages

[Julien Fischer]

Hi Zoltan,

On Fri, 27 Nov 2015, Zoltan Somogyi wrote:

> I would like the review to answer a question: should the predicates that
> convert insts to format components be put into a new module or not? There is
> another place in the compiler that could use them; the code that reports
> invalid insts in mutable declarations.

Yes, they should be moved to a separate module.

> Generate more readable mode errors.
> 
> compiler/mode_errors.m:
>     When printing insts in error messages, use predicates specifically
>     designed for that purpose.
>
>     The insts output by these predicates improve on the existing output
>     in the following ways.
>
>     - We now show the structure of complex insts through indentation.
>
>     - We now show both the name of named insts, and their expansion.
>       Sometimes, you need the expanded inst to understand the error message,
>       in the case of recursive insts, you cannot expand them forever.
>       Since the expansion *has* to refer to the inst name, showing that name
>       at the *start* of the expansion is necessary to allow readers to
>       understand the inst.
>
>     - We now eliminate the module qualification from the names of function
>       symbols. The module qualifiers on function symbols are specified
>       by the type, not the inst, of the relevant term, and if we get to
>       the point of mode analysis having been run, then the predicate
>       we are generating errors for must be *type* correct, though obviously
>       it is not *mode* correct.
>
>     - We now eliminate the module qualification from names of named insts
>       if the module qualifier is the name of the module being compiled.
>       This is a common case, and in these cases, the module qualifier
>       in the error message acts as clutter, making it *harder* to read.
>
>     - We now avoid printing parentheses that humans never need, but which
>       the parser sometimes does, and which the existing inst output predicates
>       therefore had to print.
>
>     - Higher order insts contain the modes of the function or predicate.
>       We now print the builtin modes as e.g. "in" instead of the old
>       "free >> ground".

...

> diff --git a/compiler/mode_errors.m b/compiler/mode_errors.m
> index e6dbc34..35d0953 100644
> --- a/compiler/mode_errors.m
> +++ b/compiler/mode_errors.m

...

> +%-----------------------------------------------------------------------------%
> +
> +:- func mode_to_pieces(mode_info, set(inst_name), mer_mode)
> +    = list(format_component).
> +
> +mode_to_pieces(ModeInfo, Expansions0, Mode) = Pieces :-
> +    (
> +        Mode = (InitInst0 -> FinalInst0),
> +        % XXX We should strip these wrappers everywhere in both insts,
> +        % not just at the top.
> +        ( if InitInst0 = defined_inst(typed_inst(_, SubInitInstName)) then
> +            InitInst = defined_inst(SubInitInstName)
> +        else
> +            InitInst = InitInst0
> +        ),
> +        ( if FinalInst0 = defined_inst(typed_inst(_, SubFinalInstName)) then
> +            FinalInst = defined_inst(SubFinalInstName)
> +        else
> +            FinalInst = FinalInst0
> +        ),
> +        % XXX Should we special case the expanded versions of other
> +        % "builtin" modes?

The special cases should also include ui, mdi, muo and mui.

The diff looks fine otherwise.

Julien.