Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace dynamic prec in if-then with an additional node #393

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eed3si9n
Copy link
Collaborator

Problem

#350 introduced a dynamic precedence to resolve the conflict of Scala 2 and Scala 3 grammar for if-then.

Solution

This removes the dynamic precedence and replaces it with prec.right around then side.

Note

There's a report of parser getting stuck #392 and and I was hoping that removing dynamic precedence would fix that, but it didn't seem to.

@eed3si9n eed3si9n requested review from keynmol, ckipp01 and susliko April 13, 2024 23:11
@eed3si9n
Copy link
Collaborator Author

The regression in smoke test is on BigInt.scala, which kind of looks like the following:

class A
{
  def bar(that: A): A =
    if (true) A(1)
    else 2

  def foo(that: A): (A, A) =
    if (true) 1
    else 2
}

I think it fails to parse the above because it thinks (A, A) = if (true) 1 else 2 is an infixed type, which is a separate issue we probably need to address.

@eed3si9n eed3si9n marked this pull request as draft April 14, 2024 00:40
@susliko
Copy link
Collaborator

susliko commented Jun 29, 2024

Just a note.

Running tree-sitter parse -d sample.scala.txt (sample file from the issue) gives the line where parser stucks - it's line 969.
If we remove a newline between lines 968 and 969 it stops getting stuck.

For now I had no luck in fixing this.

@eed3si9n eed3si9n force-pushed the wip/then_condition branch 4 times, most recently from 0c7adb8 to abc27f3 Compare June 30, 2024 21:10
@eed3si9n eed3si9n marked this pull request as ready for review June 30, 2024 21:14
**Problem**
350 introduced a dynamic precedence to resolve
the conflict of Scala 2 and Scala 3 grammar for if-then.

**Solution**
This removes the dynamic precedence and replaces it
with prec.right around `then` side.

**Note**
There's a report of parser getting stuck tree-sitter#392 and and
I was hoping that removing dynamic precedence would
fix that, but it didn't seem to.
@eed3si9n eed3si9n force-pushed the wip/then_condition branch from abc27f3 to 5a2a3e4 Compare June 30, 2024 21:58
@eed3si9n eed3si9n marked this pull request as draft June 30, 2024 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants