Skip to content

[5.3] Fix crash on circular reference in checkContextualRequirements() #33350

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
Aug 7, 2020

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Aug 7, 2020

Cherry pick of #32776


  • Explanation:

If a circularity issue is diagnosed while resolving a type’s generic signature, the resulting generic signature is null. Clients like type resolution must handle this case explicitly. This patch shores up a case where we did not consider this possibility. It provides a narrow fix for a class of crash-on-invalid bugs that often manifest when users are in the middle of editing code.

  • Scope: rdar://66287881 shows that users encounter this crash with some regularity while editing code.

  • Resolves: rdar://problem/66656428, rdar://64992293, rdar://66287881

  • Risk: Very Low

  • Testing: Added regression tests

  • Reviewer: @slavapestov, @CodaFi

The call to getGenericSignature() might return nullptr if we encounter
a circular reference.

Fixes <rdar://problem/64992293>.
@CodaFi CodaFi added the r5.3 label Aug 7, 2020
@CodaFi CodaFi requested a review from slavapestov August 7, 2020 01:32
@CodaFi CodaFi requested a review from a team as a code owner August 7, 2020 01:32
@CodaFi
Copy link
Contributor Author

CodaFi commented Aug 7, 2020

@swift-ci test

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

Thanks for cherry picking this! Can you drop the other commit with the regression test though? That was unrelated to this problem, I was just lazy with PR testing and didn't want to kick off smoke tests for a new PR.

@CodaFi CodaFi force-pushed the a-circle-most-vicious branch from e72365d to d5a7cc4 Compare August 7, 2020 03:08
@CodaFi
Copy link
Contributor Author

CodaFi commented Aug 7, 2020

@swift-ci test

@tkremenek tkremenek merged commit fc45397 into swiftlang:release/5.3 Aug 7, 2020
@CodaFi CodaFi deleted the a-circle-most-vicious branch August 8, 2020 17:38
@AnthonyLatsis AnthonyLatsis added swift 5.3 🍒 release cherry pick Flag: Release branch cherry picks labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants