[m-rev.] for review: better diagnostics for unbalanced parentheses

[Zoltan Somogyi]

On Wed, 30 Jul 2025 12:13:37 +1000, Peter Wang <novalazy at gmail.com> wrote:
> On Tue, 29 Jul 2025 16:48:25 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> Every text editor worth using can highlight unbalanced brackets
> automatically, and when you place the cursor on a bracket character,
> can highlight or jump to its corresponding bracket pair.

Yes, they can, and the one I use, vim, can do that too.
It is very helpful in most situations, but one thing it cannot do,
as far as I know, is jump to the open parenthesis or bracket
that *should* have been closed ten or twenty lines above,
but wasn't.

When I was working on opdb clauses in compiler/options, I wrote lots
of code that constructed lists of help text pieces, and I had many cases
of vim's syntax highlighting showing me that the next clause (as I saw it)
was part of this clause (as vim saw it), which in almost all cases was
due to a missing close parenthesis. These could be found by adding
a close parenthesis after this clause, and seeing what it matched.

But sometimes I make changes assembly-line style. When I need to make
the same change in many places, I do the change once, search for
the next place that needs that same change, and the go "././/././."
with each . repeating the same change, and each / searching for the
next place. (The double slashes indicate places that matched but
did not need the change, which is something you cannot get with
global search-and-replace.) But sometimes, the original change itself had
an unclosed parenthesis, which means that I just added tens or even a hundred
unclosed parentheses to many clauses at once (as I see them;
due to the missing parentheses, vim and mmc see them as one clause).

The new functionality in mercury_term_parser.m is designed to help
fix such issues faster by pointing out *all at once* all the instances
of the missing close parenthesis.

> I don't feel
> like more verbose output from the compiler is helpful in this case,
> but YMMV.

A month ago, until the experience described above, I would have agreed
with you. In this case, my mileage definitely does vary.

> I'll make another suggestion. For this:
> 
> unbalanced.m:038: Syntax error at token `)': expected comma, `|', `]', or an
> unbalanced.m:038:   operator.
> unbalanced.m:038:   There is an open `[' on line 37 between the open `(' on
> unbalanced.m:038:   line 36 and the `)' here.
> 
> I think this could be better:
> 
>     There is an unclosed `[' on line 37 between the `(' on line 36
>     and the `)' here.

That's a good idea. I will make the change when I have finished with
another issue, which is that the change to mercury_term_lexer.m
broke tests/feedback/mmc, but only when the workspace is compiled
in an MLDS grade (which is why my original bootchecks did not find it).

The issue is two-fold. First, mercury_term_lexer.m contains a predicate,
get_token_list_loop, that is tail recursive *only* with LCMC applied to it,
but LCMC was specified for it in Mercury.options under its old name,
lexer.m. I committed a trivial fix for this. The second issue is that even
with LCMC enabled, it does not actually make this predicate tail recursive.
I expect that it is due to the new coerce operations which are part of the code
that should be moved by LCMC to before the recursive call, because if
I move those coerces before the recursive call manually, you do get
tail recursion, which allows the test to complete without running out of stack.
However, this solution does not scale. I am now checking a change
to the code of lco.m to do this automatically.

Zoltan.