[m-rev.] for review: insts in mode error messages
jfischer at opturion.com
Sat Nov 28 21:28:44 AEDT 2015
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.
> 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.
More information about the reviews