-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Mark all unreachable executable statements in cases and source files #55588
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
Mark all unreachable executable statements in cases and source files #55588
Conversation
|
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
|
@typescript-bot perf test this |
|
Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at ea33899. You can monitor the build here. Update: The results are in! |
|
Heya @andrewbranch, I've started to run the regular perf test suite on this PR at ea33899. You can monitor the build here. Update: The results are in! |
|
@andrewbranch Here they are:
CompilerComparison 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: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@andrewbranch Here are the results of running the top-repos suite comparing Everything looks good! |
andrewbranch
left a comment
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.
Works for me. @DanielRosenwasser any objection to the API addition? I kind of like it.
|
@Andarist this looks like it's ready to go except for checking on test results. Do you want to finish this up? Otherwise I'll probably close it for housekeeping purposes. |
|
I think we just forgot about this. In microsoft/typescript-go#2067, I revamped the unreachable code detection in a way that effectively does what this PR did. I'm glad we were planning on merging this because I was having a hell of a time rationalizing the old behavior. |
…ble-lines-in-cases-and-source-files
fixes #55562