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
@Kha It is not clear to me why I had to change the following line
```
-syntax term ">>>" term : foo
+syntax term:1 ">>>":1 term : foo
```
The test breaks without it.
@Kha
I kept `TokenInfo` as is. That is, the `lbp` field is still `Option Nat`.
I changed my mind because we have the combinator `NonReservedSymbol`.
It feels weird to have a combinator that does not create a keyword,
but sets the `lbp`. Recall that it is used at `Lean/Parser/Tactic.lean`.
Other observations:
- We use symbols in auxiliary constructions (e.g., `mkAntiquot`). It
feels weird to set their `lbp` there.
- It felt weird to specify the `lbp` in places such as
```
def structCtor := parser! ident >> optional inferMod >> symbol " :: " 67
```
- We have a few parsing rules where the same symbol appears twice.
It is funny to set the `lbp` twice. Note that the approach we
discussed yesterday (retrieving the `lbp` from the `Environment`)
would not work here.
