Re: SMIE: wrote a mode for Roto, stuck on indentation

Stefan Monnier via Users list for the GNU Emacs text editor <[email protected]>
Newsgroups gmane.emacs.help
Message-ID <[email protected]>
> For the past while I’ve been trying to write a major mode for the Roto
> programming language, which is an embeddable scripting language in the
> same niche as Lua, but with Rust-ish syntax and static typing.  (See
> <https://roto.docs.nlnetlabs.nl/en/stable/>.)  It’s the first time
> I’ve ever tried to write an entire major mode and I’m struggling with
> getting SMIE to do what I want.

Note that SMIE is not a great match for languages whose syntax is
C-like and Roto seems to exhibit similar difficulties.

I implemented `sm-c-mode` (a C mode which uses SMIE as indentation
engine) mostly as a challenge, but it was not a very pleasant experience
and the result is not as good as for most other languages that SMIE.

So, SMIE's lack of polish and your lack of familiarity with SMIE are
only part of the reason for your struggles.

> Problem 1: This fragment should be indented like this:
>
>     test foo {
>         print("abcd");
>     }
>
> but C-M-\ reindents it like this:
>
>     test foo {
>         print("abcd");
>              }
>
> (There is an example in the manual that is supposed to be about
> exactly this, but it does not work for me.  If you look at
> roto--smie-rules-function, you’ll see that the
> (and (member token '("(" "[" "{")) ...) case is *almost* like that
> example, but looking for :after instead of :before; with :before it did
> absolutely nothing in my tests, with :after it corrects the
> indentation of the “print” lines but *not* the indentation of the
> close braces.)

Looking at your grammar I see:

>             ("fn"        id "(" sexps ")"         "{" sexps "}")
>             ("fn"        id "(" sexps ") ->" type "{" sexps "}")

This means that SMIE won't consider (...) and {...} as an AST node, and
it won't consider that your sexps are "inside" a pair of braces
or parentheses.  It actually won't consider "(" to be paired with ")"
and more than that "fn" is paired with "(" or that ")" is paired with "{".

For that reason, when looking at a "}" it won't call the smie-rules on
`:before "{"` because it will only try to indent it relative to the head
of the node, i.e. `fn`, or `test`, or ...

My intuition would be instead to use

    (sexp (id)
             ("{" sexps "}")
             ("(" sexps ")")
             ("fn" sexp)
             ...

and then use rules on `:before "{"` and `:before "("` to try and align
those opening element appropriately.

The problem with that approach will be to handle the optional `-> type`,
tho.  Because now `-> type` would be followed by an AST node `{...}`, so
the OPG would inevitably consider the whole `-> type {...}` as a single
node with two children (the type and the block), so now

    fn id (args) -> type { body }

would get parsed as

    (fn id (args) (-> type { body }))

Maybe you can live with it by adding a hack in the smie-rules code to
still indent `{body}` relative to `fn` despite the intermediate `->` in
the AST): basically in the `:before "{"` you'd check if the parent is
`->` and if so align with the parent's parent.

Another approach might be to add a hack in the tokenizer such that when
it sees a `{` that terminates a type, it emits two tokens, the first
being a "synthetic" token we could call "-> closer", so you'd add a rule

    ("->" type "-> closer")

to your `sexp`.  The problem with that is to make sure those two tokens
are emitted.  In [sml-mode](https://elpa.gnu.org/packages/sml-mode.html)
I faced a similar problem where the paired `local ... end` block should
also close a previous `val = exp`.
Look for `sml-smie--pending-token` in that code.

> Problem 2: If you have a bunch of things one after another they keep
> getting indented more and more, as if they were all one big expression.
>
>     test foo {
>         print("abcd");
>     }
>
>     test bar {
>         print("efgh");
>     }
>
>     test blurf {
>         print("ijkl");
>     }

Here, SMIE might need to be given extra "separator" tokens between each test.

The problematic indentation for the above case suggests that SMIE ends
up parsing the above as

    (test ... (test ... (test ...)))

Note: I talk above about AST, but of course SMIE doesn't actually build
such a thing, so in order to "see" the AST, you need to use
`smie-for/backward-sexp-command`.
[ And it's important/useful to double check if the forward and backward
  direction behave consistently: if they don't it can be a sign of
  a problem in the tokenizer.  ]

> Problem 3 (probably closely related to problem 2 but not exactly the
> same): This
>
>     test plugh {
>         let a = 123;
>         let bc = 456;
>         let def = 789;
>         let ghij = 0;
>         let klmno = 3.14159;
>     }
>
> gets reindented like this:
>
>     test plugh {
>         let a = 123;
>                 let bc = 456;
>                          let def = 789;
>                                    let ghij = 0;
>                                               let klmno = 3.14159;
>              }



> Notice here how each ‘let’ is precisely aligned with the number on
> the previous line.  It’s treating ‘123; let bc = ...’ as _all_ being
> right-hand side of the ‘let a =’.  Stefan Monnier told me on Mastodon
> that I ought to be able to fix this by adding some entries to the
> part of the grammar that’s written as a raw precedence table, so that
> SMIE knows ‘=’ binds tighter than ‘;’ (and ‘,’).  That makes sense,
> but I couldn’t make it work; if you uncomment either of the commented-
> out lines marked as “disabled due to precedence conflict,” you’ll get
> (some of) these load-time warnings:
>
>   ⛔ Warning (smie): Conflict: ; </= ;
>   ⛔ Warning (smie): Conflict: , </> ;
>   ⛔ Warning (smie): Conflict: , </= ,

These are "spurious": IIUC they're indirectly due to the fact that the

       (assoc ";")
       (assoc ",")

you pass to `smie-bnf->prec2` are not used: these are used only if
needed to disambiguate the BNF, but you're careful to write

       (sexps (sexp) (sexp ";" sexps) (sexp "," sexps))

so your BNF says they're right-associative and that introduces
no conflict.  If you replace the above with

       (sexps (sexp) (sexps ";" sexps) (sexps "," sexps))

then the BNF has an ambiguity, and then `smie-bnf->prec2` will
use the

       (assoc ";")
       (assoc ",")

to mark `; = ;` and `, = ,` and `, = ;`, and `; = ,`, which is then
compatible with the

       (assoc ";")
       (assoc ",")

your have in your `smie-precs->prec2`.

[ Sorry, I never got to make `smie-bnf->prec2` emit warnings about
  unused elements of its RESOLVERS argument, which would have been
  useful here.  ]

> Problem 4 isn’t an _indentation_ problem, but it’s a SMIE problem.
> Write
>
> test foo {}
>
> in a roto-mode buffer, enable blink-matching-paren, and put the cursor
> on any character of the word “test”.  That word will be highlighted as
> an unmatched opener.  This also happens for all the other keywords
> that introduce a construct that ends with a brace block.

Usually this means that the parsing didn't proceed the way you expect.
When I `M-x trace-function RET roto--forward-token` and then do `M-C-f`
from just before this `test foo {}` I see that your tokenizer never
returns a `{` token, instead it returns a "" token which tells SMIE to
step over the `{...}` using the syntax-table so SMIE itself will never
see the `}` token that supposedly ends that `test ...`.

IOW, you need to choose whether {...} and (...) should be parsed by the
syntax-table or by SMIE.

Parsing them via syntax-tables is the more efficient solution (and the
solution I've used in all my modes), but it forces the use of `"{"
... "}"` as a separate AST node, so `"}"` can't be the closer of `fn` or
`test`).

> p.s. Please cc: me on all replies, I'm not subscribed.

Same for me 🙂


=== Stefan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.