Skip to content

Fix crash in getAwaitedType #54107

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

Merged
merged 1 commit into from
May 3, 2023
Merged

Fix crash in getAwaitedType #54107

merged 1 commit into from
May 3, 2023

Conversation

rbuckton
Copy link
Contributor

@rbuckton rbuckton commented May 2, 2023

Fixes a crash in getAwaitedType due to a missing diagnostic message.

Fixes #53145

@rbuckton rbuckton requested review from sandersn and jakebailey May 2, 2023 20:25
@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels May 2, 2023
@jakebailey
Copy link
Member

@typescript-bot test this
@typescript-bot test top100
@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2023

Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at d7467bd. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2023

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at d7467bd. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2023

Heya @jakebailey, I've started to run the extended test suite on this PR at d7467bd. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/54107/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

@jakebailey
Copy link
Member

Well, the user deltas were supposed to show that the crash went away, but apparently the infra is totally skipping the test I added. I'll have to figure out why.

@rbuckton
Copy link
Contributor Author

rbuckton commented May 2, 2023

FYI, I did verify that the crash repro'd with the added test before applying the fix. The test I added is the smallest fragment of https://github.com/tc39/test262/blob/main/test/language/expressions/async-generator/named-yield-star-async-next.js necessary to trigger the crash.

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

I sent microsoft/typescript-error-deltas#115 which hopefully will get test262 running, but LGTM.

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the top-repos suite comparing main and refs/pull/54107/merge:

Everything looks good!

@jakebailey
Copy link
Member

@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2023

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at d7467bd. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/54107/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

@jakebailey
Copy link
Member

third time's the charm

@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 2, 2023

Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at d7467bd. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@jakebailey Here are the results of running the user test suite comparing main and refs/pull/54107/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Unknown failure"
  • 1 instance of "Package install failed"

Otherwise...

Everything looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debug assert in getAwaitedTypeNoAlias in test262 repo
4 participants