Skip to content
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

Don't add constraints for invalid subscript declarations. #389

Merged
merged 1 commit into from Dec 11, 2015

Conversation

@gregomni
Copy link
Collaborator

commented Dec 10, 2015

This fixes the crash in sr-114.

@gregomni gregomni force-pushed the gregomni:sr-114 branch Dec 10, 2015

@gregomni

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 10, 2015

Adding constraints for the invalid operator decl means constraining to the error type, which fails an assertion later on while binding an overload choice. In all the normal function/method overload choice cases, if the decl is invalid that choice gets skipped (never generated), so this is just another case of the existing way of doing things.

@lattner

This comment has been minimized.

Copy link
Collaborator

commented Dec 11, 2015

Great catch. I'd suggest returning "Type()" when the decl is known invalid (mirroring the behavior in visitOverloadedMemberRefExpr around line 1312). Also, since the test case is small and should be fast to run, please add it to the non-validation test suite, I'd suggest in test/decl/subscript/subscripting.swift.

Thank you for tracking this down and fixing it!

@lattner lattner self-assigned this Dec 11, 2015

@gregomni

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 11, 2015

Sure thing! Here's a second commit with your suggestions. If it looks good, let me know, and I'll squash the two.

Don't add constraints for invalid subscript declarations.
This fixes the crash in sr-114. Adding constraints for the invalid operator decl means constraining to the error type, which fails an
assertion later on while binding an overload choice. In all the normal function/method overload choice cases, if the decl is invalid
that choice gets skipped (never generated), so this is just another case of the existing way of doing things

@gregomni gregomni force-pushed the gregomni:sr-114 branch to 8e0b01b Dec 11, 2015

@gregomni

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 11, 2015

Actually, just went ahead and squashed because hopefully this is the final change needed.

@lattner

This comment has been minimized.

Copy link
Collaborator

commented Dec 11, 2015

Totally awesome, thank you!

lattner added a commit that referenced this pull request Dec 11, 2015

Merge pull request #389 from gregomni/sr-114
Don't add constraints for invalid subscript declarations.

@lattner lattner merged commit 8f15815 into apple:master Dec 11, 2015

@lattner

This comment has been minimized.

Copy link
Collaborator

commented Dec 11, 2015

Also, please do the honors of closing SR-114! :-)

@slavapestov

This comment has been minimized.

Copy link
Member

commented on 8e0b01b Dec 11, 2015

@gregomni Nice catch!

@gregomni gregomni deleted the gregomni:sr-114 branch May 30, 2016

slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018

Merge pull request apple#389 from compnerd/debug
build: link against swiftOnoneSupport in debug configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.