Before this commit, we were handling the following two abnormal cases:
1- `p` did not produce any `Syntax` node. `runLongestMatchParser`
would insert a `Syntax.missing`.
2- `p` produced more than one `Syntax` node on the
stack. `runLongestMatchParser` would combine all of them using a
`nullKind` node.
This feature was never used since we only use `longestMatchFn` to
process leading and trailing parsers. In both case, we create a node.
Moreover, if a bad parser is manually created, I think it is better to
fail than accept using `Syntax.missing` or a `nullKind` node. We would
only be postponing the problem since we don't have elaboration
functions for them.
cc @Kha
Several combinators (e.g., `checkNoWsBefore`) access the "leading
term" by retrieving the element on the top of the syntax stack.
However, the `longestMatchFn` was accumulating successful matches on
the syntax stack too. This commit makes sure the `longestMatchFn`
always push the "leading term" before trying a parser.
This commit also eliminates the `orelseFn` hack at `trailingLoopStep`.
Now, we use `longestMatchFn` for all parsers. Note that we have to add
a new combinator (`checkWsBeforeIfSymbol`) to make sure that `a[i]`
cannot be parsed by the `app` parsing rule.
Before removing the hack from `trailingLoopStep`,
`a[i]` was correctly parsed as an `arrayRef`. After the change,
the `app` parser is tried and it succeeds (IF we don't use
`checkWsBeforeIfSymbol`). It is an application using the list literal
`[i]`.
The `checkWsBeforeIfSymbol` ensures that `a[i]` is not a valid `app`.
cc @Kha
@Kha Note that I had to write the weird pattern
```
match_syntax stx with
| `(notation:$prec $items* => $rhs) => expandNotationAux stx prec items rhs
| `(notation $noprec* $items* => $rhs) => expandNotationAux stx none items rhs
| _ => Macro.throwUnsupported
```
with the weird `$noprec*` to match the case where the optional
precedence is not provided. I realized this is not a bug, but
I guess most users will be puzzled by this behavior. If we had a kind
for `notationItem`, I would be able to write
```
`(notation $items:notationItems* => $rhs)
```
Before this commit, if we execut `longestMatch [p1, p2]` in a
trailing parser where both `p1` and `p2` succeed, then an incorrect
syntax tree is generated by `p2`. The issue is that before executing
`p2`, the trailing stack is of the form
`#[..., left, syntax-by-p1]`
Then, the trailing parser `p2` incorrectly assumes that `syntax-by-p1`
was the "left" syntax node.
This commit fix this issue by storing the `left` node at the
`ParserContext` at `trailingLoop`
@Kha This bug was introduced when we unified leading and trailing
parsers. I think this is the simplest solution.
We can observe the bug before this commit by using
```
set_option syntaxMaxDepth 1000
set_option trace.Elab true
```
at `choiceMacroRules.lean`.
It is funny the correct result was produced even with the bug.
It was working because the `macro_rules` was matching the
buggy syntax with the nested `syntax-by-p1` there :)
@Kha I implemented the solution 1 I described at Zulip.
I also tried to document the issue, and made sure test
`choiceMacroRules` fail if the bug is reintroduced.
@Kha Could you please double check these modifications.
I added a no-op for `checkRbpLt`. It is used at the `Sort` and `Type`
parsers.
As I described in previous commits, the `checkRBPGreater` comment and
implementation were misleading. It was actually succeeding when the
rbp was less than or equal to the given parameter. So, it was renamed
to `checkRbpLe`. So, is the depArrow parenthesizer ok? I did not check.
I updated the PPRoundtrip.lean.expected.out to make sure the tests
succeed, but we should revise it if there is a problem with the
modifications at Parenthesizer.lean
