[m-rev.] for review: simplify some code in error_util.m

[Julien Fischer]

Hi Zoltan,

On Wed, 12 Oct 2022, Zoltan Somogyi wrote:

> The change that this diff prepares for has to do
> with structured output. At the moment, we indicate
> the structure of e.g. types using indentation, yielding
> output that looks like this:
>
> fbnf.m:214:   type error: variable `Entry' has type
> fbnf.m:214:     pred(
> fbnf.m:214:       fbnf.fstate :: in,
> fbnf.m:214:       fbnf.fstate :: out,
> fbnf.m:214:       io.state :: di,
> fbnf.m:214:       io.state :: uo,
> fbnf.m:214:       list.list(
> fbnf.m:214:         string
> fbnf.m:214:       ) :: in,
> fbnf.m:214:       list.list(
> fbnf.m:214:         string
> fbnf.m:214:       ) :: out
> fbnf.m:214:     ) is det,
>
> This would be easier to read if we put the list.list(string)::in
> parts on one line instead of three. Yet the code that constructs
> the format_components that this output is derived from cannot
> make such decisions, because it does not know (a) how long
> the context is, (b) how much indentation there will be before
> each line, and (c) how long the text of each line is. It could
> possibly be taught to answer all three questions, but only
> at the cost of effectively reimplementing the part of error_util.m
> that writes out error_specs, which does not strike me as
> a good idea.
>
> Instead, I intend to get the code that generates these
> format_components to replace the parentheses at the end of
> "list.list(" and at the start of ") :: in" with non-printable
> characters that cannot occur in error_specs. The code in
> error_util.m will look for situations in which
>
> - a line ends in the non-printable char representing
>  a collapsable (,
> - there is a later line that starts with the non-printable char
>  representing a collapsable ),
> - these two lines and the lines between them fit on one line, and
> - any collapsable paren chars occurring in the lines between
>  occur in matching pairs, i.e. these parentheses balance.
>
> If all these conditions are met, we can squash those lines onto one.
> And whether or not the conditions are met, replace the non-printable
> collabsable paren characters with ( and ).
>
> Can anyone see a problem with this plan? Or a better plan?

Why not just introduce new format components, e.g. collapsible_lparen
and collapsible_rparen, that do what you want?  That seems as though
it would avoid a bunch of fiddly string manipulation, which encoding
the structure using non-printable characters would not.

> I would also like to break up error_util.m, since its cohesion is
> getting worse and worse. I propose that
>
> - a new module called error_spec should contain the error_spec
>  type and its components (most other compiler modules will need
>  to import just this module),
>
> - a new module called maybe write_error_spec would contain
>  the code for writing out error specs, and converting them to
>  string, and
>
> - error_util itself should retain the utility predicates, such as
>  type_to_pieces.
>
> I am not sure where the functions operating on error_severities
> should go; I will see where they fit.
>
> Any objections, or ideas for better module names?

No objections from me.

The diff looks fine.

Julien.