NomicFoundation / slang

Solidity compiler tooling by @NomicFoundation
https://nomicfoundation.github.io/slang/
MIT License
218 stars 20 forks source link

Unify greedy scanners #608

Open AntonyBlakey opened 9 months ago

AntonyBlakey commented 9 months ago

I suggest we treat terminators and separators as delimiters with an empty opening delimiter and hence unify the three error correction strategies

Xanewok commented 8 months ago

For context, in terms of greedy scan logic itself:

  1. both DelimitedBy and TerminatedBy use ParserResult::recover_until_with_nested_delims
  2. SeparatedBy uses a dedicated SeparatedHelper::run

which all use skip_until_with_nested_delims underneath for the actual skipping/greedy scan, so these are unified in that sense.

I'll see if I can simplify the SeparatedBy to not require that much extra code (in that case I think we'd benefit from https://github.com/NomicFoundation/slang/issues/600) but the question remains for the TerminatedBy.

If we change it so that we stop the greedy scan on the terminator as well, we might end up with imbalanced delimiters, since, for example, there may be a partial match that eats an opening delimiter which we will attempt to recover at an inner semicolon and not skip over the closing delimiter as the result. We only keep a local stack of delimiters during a recovery itself, so we still might accept the terminator

I've opened a https://github.com/Xanewok/slang/tree/stop-skipping-on-terminator as PoC (see here how it affects the { unchecked { x = 1; } } example).

One idea that pops to my head is that we could modify the parser to keep the delimiter stack for the parse and not only during recovery - we would accept terminator only if the parsed delimiters are not unclosed. I'll have to think a bit more about this.