-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Perform bounds-checking on string input in scanner #58362
base: main
Are you sure you want to change the base?
Conversation
@typescript-bot perf test |
@rbuckton, the perf run you requested failed. You can check the log here. |
1 similar comment
@rbuckton, the perf run you requested failed. You can check the log here. |
@typescript-bot perf test |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@rbuckton Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Looks like there was no noticeable performance improvement with this change, so we should only take it if we think the bounds checking enforcement is worth adding purely for the sake of correctness and stability. |
It seems like parse is (only very slightly) worse - I wonder what happens if you switch that initial call in |
It may be that some code paths aren't run enough times to be optimized and thus the functions aren't inlined. I'll have to look at it in deopt-explorer. |
Following on to #58339, this performs the same bounds checking throughout most of the rest of the scanner.
This also adds the
local/bounds-check
lint rule to warn if future changes incorrectly perform unchecked calls tocharCodeAt
orcodePointAt
ontext
. The lint rule performs some rudimentary analysis to determine whether a call tocodePointUnchecked
orcharCodeUnchecked
follows a valid boundary check to cut down on excess// eslint-disable-line
comments for cases like these: