Skip to content

[Sema] Fix rdar://75219757 #36493

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

Closed
wants to merge 1 commit into from
Closed

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Mar 18, 2021

I don’t actually know what I’m changing here, but #36028 appears to have been to optimistic to declare the restored branch as unreachable, which causes a crash in the added test case. I’m just restoring the old behavior to prevent the crash. Please let me know if you think the code path should really be unreachable and we need to fix a deeper issue the way code completion sets up the constraint system.

Fixes rdar://75219757 [SR-14319]

It appears like the simplification in 9d3c8ca was a little over-optimistic to declare the code path unreachable and causes a crash in the test case added by this commit.

Restore the old code code path.

Fixes rdar://75219757
@ahoppen ahoppen requested a review from slavapestov March 18, 2021 16:42
@ahoppen
Copy link
Member Author

ahoppen commented Mar 18, 2021

@swift-ci Please smoke test

@ahoppen
Copy link
Member Author

ahoppen commented Mar 19, 2021

@swift-ci Please smoke test Linux

auto selected = solution.getOverloadChoice(memberLocator);
auto selectedElt = solution.getOverloadChoiceIfAvailable(memberLocator);

if (!selectedElt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is restoring yet another path which was removed by #36028 - I'd suggest to do a revert and add test cases from #36494 and here in a separate commit to make sure we get a crash next time we attempt to remove this code and code completion still relies on broken AST...

@ahoppen
Copy link
Member Author

ahoppen commented Mar 19, 2021

Closing this to revert #36028 in #36523.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants