-
Notifications
You must be signed in to change notification settings - Fork 465
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 reserved words #6831
fix reserved words #6831
Conversation
I just modified the test and now they only throw a
Others should be fine to allow. |
0cb1079
to
51a9d65
Compare
I removed the codegen script at b1275cb since now we can manage keywords manually. I ran the syntax benchmark and there were no noticeable perf changes by using OCaml Set instead of binary search. |
I noticed that it isn't BST. Also, it is a list that can never be changed, it does not benefit from using an immutable tree. I replaced it with a Hashtbl to optimize lookups. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks a lot!
The compiler prefixes conflicting ident names with
$$
to prevent the output from containing a SyntaxError or unintentionally declarying conflicting names.However, some of the managed keywords are outdated (keywords listed on the Future reserved words in older standards section), some are not actually keywords, and the list of Chrome global variables is far too extensive.
Changes
Hashtbl
instead of binary search (keywords are small enough)Future works
If a name is not an actual JS keyword, there should be no problem defining the identifier with the name inside the module scope, even if it is used in the
globalThis
scope. But theSyntaxError
only occurs in the CommonJS system.IMHO we need a way to allow using those names if users want. In a previous PR I proposed a syntax using exotic ident.
However, this cannot be implemented in the current parser. If necessary, the scanner would have to be modified to keep exotic tokens, which would significantly increase the complexity of printers.