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

wrong ppx #1063

Closed
bobzhang opened this issue Jan 8, 2017 · 5 comments
Closed

wrong ppx #1063

bobzhang opened this issue Jan 8, 2017 · 5 comments
Labels
stale Old issues that went stale

Comments

@bobzhang
Copy link
Member

bobzhang commented Jan 8, 2017

let bla foo bar baz = foo##(bar##baz)

This should fail to compile instead of weird output

@bobzhang bobzhang added the bug label Jan 8, 2017
@bobzhang
Copy link
Member Author

bobzhang commented Jan 8, 2017

Here is an explanation whyfoo##(bar##baz) works
bucklescript allows chaining as below:

foo##(method1 a b )##(method2 a b)

which is the same as

let result = foo##method1 a b in 
result##method2 a b 

foo##(bar##baz) is parsed as

foo##( (##) bar baz)

relevant reasonml/reason#941

@bobzhang
Copy link
Member Author

bobzhang commented Jan 8, 2017

proposal fix: do a sanity check, don't allow method name to have #?
cc @chenglou

@bobzhang
Copy link
Member Author

bobzhang commented Jan 8, 2017

actually the right fix is apply ast rewrite recursively first in the inner ast, then we get a more sensible error message

@bobzhang
Copy link
Member Author

bobzhang commented Jan 9, 2017

current rewrite rules
Pexp_apply(fn,args) pattern math fn and args before it is rewritten, this cause some confusion. a more polished fix is always rewrote subexpression before pattern match.
This is not a critical bug, let's leave it for next release

@stale
Copy link

stale bot commented May 12, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Old issues that went stale label May 12, 2023
@stale stale bot closed this as completed May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Old issues that went stale
Projects
None yet
Development

No branches or pull requests

1 participant