Skip to content

[ClangImporter] Don't crash when a bad override affects NSErrors. #7936

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

Conversation

jrose-apple
Copy link
Contributor

  • Explanation: When importing an Objective-C method, the compiler has to decide whether it should be imported using throws. However, when a method is an override, it skips all that work and assumes the decisions made for the superclass method apply here as well. This can really throw things off if the types don't match up, though. Handle the one case where this is legal according to the rules of Objective-C, and make sure we don't import methods in the other cases.
  • Scope: Affects importing of Objective-C error-throwing methods with incorrect signatures that override those with correct signatures. The most common case of this would be incorrect nullability on the return type.
  • Issue: rdar://problem/30705461
  • Reviewed by: @DougGregor
  • Risk: Very low. All existing correct code will continue to do exactly what it did before; common cases of incorrect code now produce reasonable behavior instead of crashing the compiler.
  • Testing: Added compiler regression tests.

…iftlang#7907)

Most of the time the name importer does a good job deciding whether to
import a particular method as throwing or not. However, when a method
is an override, it skips all that work and assumes the decisions made
for the superclass method apply here as well---which makes sense,
since you're going to get the subclass implementation if you call the
superclass's entry point. This can really throw things off if the
types /don't/ match up, though. Handle the one case where this is
legal according to the rules of Objective-C, and make sure we don't
import methods in the other cases.

rdar://problem/30705461
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@DougGregor DougGregor added this to the Swift 3.1 milestone Mar 6, 2017
@DougGregor
Copy link
Member

LGTM!

@ematejska ematejska merged commit ebaa82c into swiftlang:swift-3.1-branch Mar 6, 2017
@jrose-apple jrose-apple deleted the 3.1-undeserved-assertions-for-erroneous-errors branch March 6, 2017 21:42
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.

4 participants