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

Fix error when parsing let x=-.1; and co. #1945

Merged
merged 3 commits into from
May 30, 2018

Conversation

anmonteiro
Copy link
Member

This is a follow-up to #1642 employing the strategy highlighted in the following
comment: #1642 (comment)

It removes the special case around LABEL_WITH_EQUAL, making other valid inputs
parseable too.

cc @IwanKaramazow

@anmonteiro anmonteiro changed the title Fix syntax error when parsing let x=-.1; and co. Fix error when parsing let x=-.1; and co. May 20, 2018
@IwanKaramazow
Copy link
Contributor

IwanKaramazow commented May 20, 2018

Now that I think of it, can you add a test to verify that =+ etc. as infix operators are still usable?

@anmonteiro
Copy link
Member Author

Yeah I think I might have broken those.

I'll add tests and make sure they're working. Thanks for the feedback

@anmonteiro
Copy link
Member Author

This is actually trickier than I envisioned, and possibly even unsolvable.

Imagine the following:

let (=+) = (a, b) => a + b;
let (=+-) = (a, b) => b;

let foo = a =+-b;

now what is really foo? It could either be parsed as a =+- b or a =+ (-b)

@anmonteiro
Copy link
Member Author

I think the answer to my question is infix precedence.

I managed to fix this issue in the end and added some regression tests around it.

This was definitely a tricky one, and I think we can make it work!

@@ -353,18 +353,6 @@ let () =
None
)

(* To "unlex" a few characters *)
let set_lexeme_length buf n = (
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's another one around line 190

@chenglou
Copy link
Member

Man I wish we can disable userland infix operator one day, and then provide a set of e.g. 5-10 good ones, whose precedence we can hard-code, and whose symbol won't be constrained by the first char.

Anyway, is this diff ready?

@anmonteiro
Copy link
Member Author

I really agree with that "first character" statement.

Yeah this is ready to go. It's unfortunate that it complicates the grammar a little bit but both Iwan and I can't see another immediate way to support this case.

This is a follow-up to reasonml#1642 employing the strategy highlighted in the following
comment: reasonml#1642 (comment)

It removes the special case around `LABEL_WITH_EQUAL`, making other valid inputs
parseable too.
also add regression tests for these cases
@chenglou
Copy link
Member

Alright then, merging this one. Thanks for the hard work! Doesn't seem trivial

@chenglou chenglou merged commit 95f027d into reasonml:master May 30, 2018
@anmonteiro anmonteiro deleted the gh-1642-follow-up branch May 30, 2018 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants