-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
Strict Mode Lex/Parse #717
Conversation
Codecov Report
@@ Coverage Diff @@
## master #717 +/- ##
==========================================
+ Coverage 59.13% 59.20% +0.06%
==========================================
Files 155 155
Lines 9838 9905 +67
==========================================
+ Hits 5818 5864 +46
- Misses 4020 4041 +21
Continue to review full report at Codecov.
|
Benchmark for 70bd6f4Click to view benchmark
|
Benchmark for e52f418Click to view benchmark
|
Benchmark for e2c867dClick to view benchmark
|
One thing that is needed is a way for knowing if we are in strict mode from the parser, since we do need to change some parsing semantics. |
For the parser this should be fine as the use strict directive is read at the parser level, my thought was that we can pass strict_mode as an argument between parse functions with a flag on the parser used if global strict mode is enabled. What might be more complicated is enforcing stuff like "strict mode makes attempts to delete undeletable properties throw" as we may not be able to do this during parsing. |
We already pass the lexer as an argument, and the strict mode is relevant when lexing. I would add a
I think that we should have a If I understand the strict mode properly, it starts to take effect once it is declared, but I'm not sure about the scope. If it has a function scope, then the parser must be able to set it to false after parsing the function, and the executor should be able to check the string literal to set its own flag on execution. |
Thinking about this more I see the benefit of using a flag on lexer. Ignore below I put strict_mode as a flag to lex() which I think might work better than trying to make sure we set a flag on the parser when we exit function scope. This way if the entire script is strict mode then we just set it to true at the top level and it carries through and if only a function is strict mode then we just set the flag to true when parsing the function statement list and this way it won't accidentally apply to too wide a scope. Also to me having it included in lex/parse means that if someone later wants to add another thing to lex/parse it is apparent that the strict mode case needs to be handled and how to do so is easy to understand. |
Benchmark for 238662eClick to view benchmark
|
Benchmark for b359035Click to view benchmark
|
Benchmark for 6f794bcClick to view benchmark
|
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.
Looks good to me (other than the forgotten dbg!()
), but it's the first time I look inside the syntax
module, definitely wait for someone who knows more about it.
Benchmark for 29b89a7Click to view benchmark
|
Benchmark for 5b6573aClick to view benchmark
|
Benchmark for 7f93367Click to view benchmark
|
Benchmark for 85d8289Click to view benchmark
|
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.
Looks perfect to me!
Benchmark for 9bd3421Click to view benchmark
|
Benchmark for 2fcff9aClick to view benchmark
|
This Pull Request addresses #716.
It changes the following: