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 SHARPOP precedence #2050

Merged
merged 3 commits into from
Jul 5, 2018
Merged

Fix SHARPOP precedence #2050

merged 3 commits into from
Jul 5, 2018

Conversation

anmonteiro
Copy link
Member

@anmonteiro anmonteiro commented Jul 5, 2018

fixes #1507 #774

Not really sure how to write tests for this one.

I verified the following expressions manually:

$ echo 'foo##bar.baz' | ./_build/install/default/bin/refmt -p ml
;;(foo ## bar).baz

$ echo 'foo##bar[1]' | ./_build/install/default/bin/refmt -p ml
;;(foo ## bar).(1)

$ echo 'foo##bar[0]##baz[1]' | ./_build/install/default/bin/refmt -p ml
;;(((foo ## bar).(0)) ## baz).(1)

@anmonteiro
Copy link
Member Author

A potential problem might be:

$ echo 'foo#= bar[0]' | ./_build/install/default/bin/refmt -p ml
;;(foo #= bar).(0)

I don't think this is right, but we can special case it in the lexer.

@anmonteiro
Copy link
Member Author

Just fixed the issue pointed out in my last comment with the latest commit.

@@ -1209,6 +1209,7 @@ let package_type_of_module_type pmty =
%token SEMISEMI
%token SHARP
%token <string> SHARPOP
%token SHARPOP_WITH_EQUAL
Copy link
Contributor

Choose a reason for hiding this comment

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

can you rename this to SHARPEQUAL? SHARPOP_WITH_EQUAL conveys # with = , if you look at the other token names.

Copy link
Member Author

Choose a reason for hiding this comment

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

definitely

}
| "#" operator_chars+
{ SHARPOP(lexeme_operator lexbuf) }
{ let l = lexeme_operator lexbuf in
if l = "#=" then
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should add | "#=" { SHARPEQUAL } for clarity in the lexer. No hard opinions here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's better too, will incorporate.

@anmonteiro
Copy link
Member Author

Just pushed the suggested changes.

@chenglou
Copy link
Member

chenglou commented Jul 5, 2018

M-m-merged!

@chenglou chenglou merged commit 499bf19 into reasonml:master Jul 5, 2018
@anmonteiro anmonteiro deleted the gh-1507-774 branch July 5, 2018 18:29
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Jul 6, 2018
…ce, parens

unnecessary on the right side
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Jul 6, 2018
…ce, parens

unnecessary on the right side
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Jul 10, 2018
…ce, parens

unnecessary on the right side
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Jul 22, 2018
…ce, parens

unnecessary on the right side
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Aug 10, 2018
…ce, parens

unnecessary on the right side
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Oct 2, 2018
…ce, parens

unnecessary on the right side
anmonteiro added a commit to anmonteiro/reason that referenced this pull request Oct 31, 2018
…ce, parens

unnecessary on the right side
chenglou pushed a commit that referenced this pull request Nov 11, 2018
* Syntax sugar `["literal_string"]` for SHARPOPs `##` and `#=`

* rebase on master after #2050, parse `#=` with higher precedence, parens
unnecessary on the right side

* array, string and bigarray get also get `#=` -> `=`

* rebase on fast pipe fix

* small refactor

* Fix printer after rebase (caused by earlier refactor)

* fix tests after rebase

* Fixes after rebase
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.

location##streets[1] precedence is wrong
4 participants