Skip to content

Add error message for keywords with escapes in them #32718

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Aug 12, 2019
Merged

Add error message for keywords with escapes in them #32718

merged 12 commits into from
Aug 12, 2019
+341 −12

Conversation

weswigham
Copy link
Member

Fixes #32700

weswigham
weswigham commented Aug 5, 2019
View reviewed changes
tests/baselines/reference/switchStatementsWithMultipleDefaults.errors.txt Outdated
break;
case 10000:
x /= x;
default: // Error, third 'default' clause
def\u0061ult: // Error, fourth 'default' clause.
~~~~~~~~~~~~
!!! error TS1260: Keyword must not contain escaped characters
Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, it is correct to issue an error here. You can paste const a = {def\u0061ult: 12} into the repl and be greeted with an error. It surprised me, tbh.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fun fact: After some discussion with @rbuckton, while this errors in chrome (therefore v8), it does not in firefox and edge - per spec it doesn't seem like it should be an error, either (since anywhere an identifier is parse-able it should work). A crbug has been logged

Copy link
Member Author

Choose a reason for hiding this comment

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

@rbuckton checked jsc as well, which matches chrome's behavior - I've opened a bug there, too.

Copy link
Member

Choose a reason for hiding this comment

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

I dug through test262 but couldn't find a test for this case. The closest I could find was for a BindingIdentifier, not an IdentifierName:

@weswigham
Move check into parser during advance to next token to utilize contex…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
e9a92f1 
…t for contextual keywords
@weswigham
git add .
dd21525
@weswigham
Merge branch 'master' into add-error-on-escaped-keywords
2ba05e6
@weswigham
Add tests for extended escapes
8b4f232
@weswigham
Copy link
Member Author

@RyanCavanaugh wanna look over this again?

@weswigham weswigham requested a review from rbuckton August 6, 2019 23:17
rbuckton
rbuckton requested changes Aug 12, 2019
View reviewed changes
@weswigham
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 12, 2019 •
edited
Loading

Heya @weswigham, I've started to run the perf test suite on this PR at d6ab219. You can monitor the build here. It should now contribute to this PR's status checks.

rbuckton
rbuckton reviewed Aug 12, 2019
View reviewed changes
@weswigham
Copy link
Member Author

@typescript-bot perf test this instead

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 12, 2019 •
edited
Loading

Heya @weswigham, I've started to run the perf test suite on this PR at 4657e04. You can monitor the build here. It should now contribute to this PR's status checks.

Update: The results are in!

@weswigham
Copy link
Member Author

@rbuckton done~

@typescript-bot
Copy link
Collaborator

@weswigham
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..32718

Metric master 32718 Delta Best Worst
Angular - node (v12.1.0, x64)
Memory used 325,353k (± 0.04%) 325,767k (± 0.02%) +415k (+ 0.13%) 325,630k 325,993k
Parse Time 1.43s (± 0.26%) 1.46s (± 0.58%) +0.02s (+ 1.67%) 1.44s 1.47s
Bind Time 0.75s (± 0.63%) 0.75s (± 0.59%) -0.00s (- 0.13%) 0.74s 0.76s
Check Time 4.20s (± 0.41%) 4.21s (± 0.45%) +0.01s (+ 0.26%) 4.18s 4.26s
Emit Time 5.28s (± 0.89%) 5.26s (± 0.85%) -0.02s (- 0.36%) 5.16s 5.36s
Total Time 11.67s (± 0.46%) 11.68s (± 0.44%) +0.01s (+ 0.09%) 11.58s 11.83s
Monaco - node (v12.1.0, x64)
Memory used 345,893k (± 0.02%) 345,916k (± 0.02%) +23k (+ 0.01%) 345,860k 346,108k
Parse Time 1.19s (± 0.68%) 1.22s (± 0.82%) +0.02s (+ 2.01%) 1.20s 1.24s
Bind Time 0.68s (± 1.14%) 0.67s (± 0.55%) -0.00s (- 0.59%) 0.67s 0.68s
Check Time 4.26s (± 0.32%) 4.26s (± 0.38%) +0.00s (+ 0.09%) 4.24s 4.31s
Emit Time 2.85s (± 0.85%) 2.85s (± 0.42%) -0.01s (- 0.25%) 2.82s 2.88s
Total Time 8.98s (± 0.27%) 9.00s (± 0.31%) +0.02s (+ 0.17%) 8.97s 9.11s
TFS - node (v12.1.0, x64)
Memory used 301,419k (± 0.02%) 301,462k (± 0.02%) +43k (+ 0.01%) 301,344k 301,574k
Parse Time 0.92s (± 0.52%) 0.93s (± 0.80%) +0.02s (+ 1.74%) 0.92s 0.95s
Bind Time 0.63s (± 3.15%) 0.62s (± 0.89%) -0.01s (- 1.74%) 0.61s 0.63s
Check Time 3.87s (± 0.46%) 3.86s (± 0.56%) -0.01s (- 0.31%) 3.83s 3.91s
Emit Time 2.96s (± 0.91%) 2.95s (± 0.58%) -0.01s (- 0.34%) 2.92s 2.99s
Total Time 8.38s (± 0.48%) 8.36s (± 0.43%) -0.02s (- 0.23%) 8.29s 8.46s
Angular - node (v8.9.0, x64)
Memory used 344,013k (± 0.02%) 344,371k (± 0.02%) +358k (+ 0.10%) 344,246k 344,556k
Parse Time 1.94s (± 0.46%) 1.96s (± 0.36%) +0.01s (+ 0.72%) 1.95s 1.98s
Bind Time 0.82s (± 0.57%) 0.82s (± 0.54%) -0.00s (- 0.24%) 0.81s 0.83s
Check Time 5.01s (± 0.61%) 5.03s (± 0.58%) +0.02s (+ 0.32%) 4.97s 5.09s
Emit Time 6.08s (± 0.88%) 6.08s (± 0.84%) -0.01s (- 0.12%) 5.96s 6.17s
Total Time 13.85s (± 0.47%) 13.88s (± 0.45%) +0.03s (+ 0.18%) 13.73s 14.01s
Monaco - node (v8.9.0, x64)
Memory used 363,416k (± 0.01%) 363,420k (± 0.01%) +4k (+ 0.00%) 363,344k 363,507k
Parse Time 1.53s (± 0.55%) 1.55s (± 0.43%) +0.02s (+ 1.18%) 1.54s 1.56s
Bind Time 0.88s (± 0.80%) 0.88s (± 0.38%) +0.00s (+ 0.11%) 0.87s 0.89s
Check Time 5.28s (± 0.77%) 5.21s (± 1.48%) -0.07s (- 1.36%) 4.99s 5.34s
Emit Time 2.92s (± 0.25%) 3.00s (± 3.92%) +0.08s (+ 2.67%) 2.90s 3.33s
Total Time 10.61s (± 0.46%) 10.63s (± 0.56%) +0.03s (+ 0.25%) 10.54s 10.78s
TFS - node (v8.9.0, x64)
Memory used 317,373k (± 0.01%) 317,389k (± 0.01%) +16k (+ 0.01%) 317,319k 317,497k
Parse Time 1.23s (± 0.42%) 1.25s (± 0.38%) +0.02s (+ 1.30%) 1.24s 1.26s
Bind Time 0.66s (± 0.72%) 0.66s (± 0.87%) 0.00s ( 0.00%) 0.65s 0.67s
Check Time 4.51s (± 0.64%) 4.53s (± 0.88%) +0.02s (+ 0.55%) 4.46s 4.62s
Emit Time 3.08s (± 0.35%) 3.08s (± 0.65%) -0.00s (- 0.00%) 3.04s 3.13s
Total Time 9.48s (± 0.34%) 9.52s (± 0.46%) +0.04s (+ 0.46%) 9.43s 9.62s
Angular - node (v8.9.0, x86)
Memory used 194,891k (± 0.02%) 195,051k (± 0.02%) +160k (+ 0.08%) 194,948k 195,124k
Parse Time 1.89s (± 0.79%) 1.89s (± 0.63%) -0.00s (- 0.05%) 1.87s 1.93s
Bind Time 0.95s (± 0.52%) 0.95s (± 0.59%) +0.00s (+ 0.21%) 0.94s 0.96s
Check Time 4.59s (± 0.72%) 4.62s (± 0.64%) +0.02s (+ 0.48%) 4.55s 4.67s
Emit Time 5.85s (± 1.05%) 5.83s (± 1.09%) -0.02s (- 0.33%) 5.72s 5.98s
Total Time 13.27s (± 0.57%) 13.28s (± 0.41%) +0.01s (+ 0.06%) 13.20s 13.41s
Monaco - node (v8.9.0, x86)
Memory used 203,053k (± 0.01%) 203,044k (± 0.02%) -9k (- 0.00%) 202,977k 203,131k
Parse Time 1.60s (± 1.04%) 1.59s (± 0.67%) -0.00s (- 0.06%) 1.58s 1.63s
Bind Time 0.72s (± 1.25%) 0.71s (± 0.84%) -0.01s (- 1.26%) 0.69s 0.72s
Check Time 4.89s (± 0.54%) 4.87s (± 0.39%) -0.03s (- 0.55%) 4.81s 4.89s
Emit Time 3.19s (± 0.57%) 3.21s (± 0.83%) +0.02s (+ 0.78%) 3.17s 3.27s
Total Time 10.39s (± 0.46%) 10.38s (± 0.46%) -0.01s (- 0.11%) 10.26s 10.47s
TFS - node (v8.9.0, x86)
Memory used 178,327k (± 0.01%) 178,324k (± 0.01%) -4k (- 0.00%) 178,253k 178,366k
Parse Time 1.30s (± 0.70%) 1.31s (± 0.55%) +0.01s (+ 0.85%) 1.30s 1.33s
Bind Time 0.63s (± 1.34%) 0.63s (± 1.59%) -0.00s (- 0.32%) 0.61s 0.65s
Check Time 4.31s (± 0.52%) 4.31s (± 0.56%) +0.00s (+ 0.09%) 4.26s 4.37s
Emit Time 2.88s (± 1.28%) 2.89s (± 0.95%) +0.01s (+ 0.42%) 2.84s 2.96s
Total Time 9.12s (± 0.46%) 9.14s (± 0.40%) +0.02s (+ 0.26%) 9.06s 9.21s
Angular - node (v9.0.0, x64)
Memory used 343,645k (± 0.01%) 344,009k (± 0.02%) +365k (+ 0.11%) 343,891k 344,216k
Parse Time 1.67s (± 0.46%) 1.69s (± 0.68%) +0.02s (+ 1.14%) 1.67s 1.71s
Bind Time 0.77s (± 0.72%) 0.77s (± 0.80%) +0.00s (+ 0.39%) 0.76s 0.78s
Check Time 4.75s (± 0.37%) 4.78s (± 0.52%) +0.02s (+ 0.53%) 4.71s 4.82s
Emit Time 5.75s (± 1.32%) 5.68s (± 1.73%) -0.07s (- 1.30%) 5.53s 5.87s
Total Time 12.93s (± 0.64%) 12.91s (± 0.89%) -0.03s (- 0.20%) 12.68s 13.16s
Monaco - node (v9.0.0, x64)
Memory used 363,544k (± 0.01%) 363,424k (± 0.02%) -120k (- 0.03%) 363,311k 363,568k
Parse Time 1.29s (± 0.56%) 1.31s (± 0.52%) +0.02s (+ 1.32%) 1.28s 1.31s
Bind Time 0.85s (± 1.10%) 0.84s (± 2.10%) -0.02s (- 1.88%) 0.81s 0.87s
Check Time 4.90s (± 1.53%) 5.00s (± 1.62%) +0.10s (+ 2.04%) 4.83s 5.13s
Emit Time 3.29s (± 3.05%) 3.11s (± 5.73%) -0.18s (- 5.56%) 2.85s 3.38s
Total Time 10.34s (± 0.62%) 10.26s (± 1.24%) -0.08s (- 0.78%) 10.02s 10.48s
TFS - node (v9.0.0, x64)
Memory used 317,389k (± 0.02%) 317,443k (± 0.01%) +55k (+ 0.02%) 317,401k 317,530k
Parse Time 1.01s (± 0.37%) 1.03s (± 0.71%) +0.02s (+ 1.67%) 1.02s 1.05s
Bind Time 0.62s (± 1.13%) 0.62s (± 0.80%) -0.00s (- 0.48%) 0.61s 0.63s
Check Time 4.39s (± 0.66%) 4.39s (± 0.61%) -0.01s (- 0.18%) 4.31s 4.45s
Emit Time 3.16s (± 2.50%) 3.22s (± 0.51%) +0.06s (+ 1.90%) 3.19s 3.25s
Total Time 9.19s (± 1.01%) 9.25s (± 0.34%) +0.07s (+ 0.71%) 9.18s 9.31s
Angular - node (v9.0.0, x86)
Memory used 194,995k (± 0.02%) 195,186k (± 0.02%) +191k (+ 0.10%) 195,082k 195,252k
Parse Time 1.60s (± 0.50%) 1.61s (± 0.74%) +0.01s (+ 0.88%) 1.59s 1.65s
Bind Time 0.89s (± 1.14%) 0.88s (± 0.51%) -0.00s (- 0.45%) 0.87s 0.89s
Check Time 4.26s (± 0.46%) 4.26s (± 0.61%) -0.00s (- 0.05%) 4.21s 4.32s
Emit Time 5.53s (± 0.46%) 5.51s (± 0.70%) -0.02s (- 0.33%) 5.42s 5.62s
Total Time 12.28s (± 0.26%) 12.27s (± 0.41%) -0.01s (- 0.09%) 12.17s 12.38s
Monaco - node (v9.0.0, x86)
Memory used 203,176k (± 0.02%) 203,169k (± 0.02%) -7k (- 0.00%) 203,090k 203,255k
Parse Time 1.32s (± 0.75%) 1.34s (± 0.50%) +0.02s (+ 1.36%) 1.33s 1.36s
Bind Time 0.64s (± 1.36%) 0.64s (± 1.48%) +0.01s (+ 0.94%) 0.63s 0.67s
Check Time 4.70s (± 0.64%) 4.74s (± 0.49%) +0.04s (+ 0.89%) 4.67s 4.79s
Emit Time 3.10s (± 0.78%) 3.10s (± 0.62%) +0.00s (+ 0.13%) 3.06s 3.16s
Total Time 9.75s (± 0.55%) 9.82s (± 0.34%) +0.07s (+ 0.70%) 9.74s 9.89s
TFS - node (v9.0.0, x86)
Memory used 178,478k (± 0.01%) 178,480k (± 0.02%) +2k (+ 0.00%) 178,430k 178,592k
Parse Time 1.04s (± 0.46%) 1.05s (± 1.02%) +0.01s (+ 1.16%) 1.02s 1.07s
Bind Time 0.58s (± 0.69%) 0.57s (± 1.30%) -0.01s (- 1.04%) 0.56s 0.60s
Check Time 4.14s (± 0.55%) 4.13s (± 0.64%) -0.01s (- 0.17%) 4.07s 4.18s
Emit Time 2.79s (± 0.78%) 2.78s (± 0.96%) -0.01s (- 0.43%) 2.71s 2.82s
Total Time 8.55s (± 0.39%) 8.54s (± 0.55%) -0.02s (- 0.20%) 8.43s 8.62s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-142-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
  • node (v9.0.0, x64)
  • node (v9.0.0, x86)
Scenarios
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Angular - node (v9.0.0, x64)
  • Angular - node (v9.0.0, x86)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • Monaco - node (v9.0.0, x64)
  • Monaco - node (v9.0.0, x86)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • TFS - node (v9.0.0, x64)
  • TFS - node (v9.0.0, x86)
Benchmark Name Iterations
Current 32718 10
Baseline master 10

@weswigham
Copy link
Member Author

Perf results are negligible - nice.

@weswigham weswigham merged commit 4ab85bb into microsoft:master Aug 12, 2019
@weswigham weswigham deleted the add-error-on-escaped-keywords branch August 12, 2019 23:00
@Kingwl Kingwl mentioned this pull request Aug 16, 2019
Merged
@DanielRosenwasser DanielRosenwasser mentioned this pull request Apr 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielRosenwasser DanielRosenwasser

@RyanCavanaugh RyanCavanaugh

@Kingwl Kingwl

@ajafff ajafff

@rbuckton rbuckton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing error for keywords with escaped characters
7 participants
@weswigham @typescript-bot @DanielRosenwasser @rbuckton @RyanCavanaugh @Kingwl @ajafff