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

[SR-11905] Objective-C interop allows creation of undefined behavior #54322

Open
DevAndArtist mannequin opened this issue Dec 5, 2019 · 5 comments
Open

[SR-11905] Objective-C interop allows creation of undefined behavior #54322

DevAndArtist mannequin opened this issue Dec 5, 2019 · 5 comments

Comments

@DevAndArtist
Copy link
Mannequin

@DevAndArtist DevAndArtist mannequin commented Dec 5, 2019

Previous ID SR-11905
Radar rdar://problem/57711780
Original Reporter @DevAndArtist
Type Bug
Environment

Apple Swift version 5.1.2 (swiftlang-1100.0.278 clang-1100.0.33.9)

Additional Detail from JIRA
Votes 1
Component/s Compiler
Labels Bug, StarterBug
Assignee faical (JIRA)
Priority Medium

md5: df1a976b8eb12c28ba3696248c7605f6

Issue Description:

In pure Swift this wouldn't be possible because we shouldn't be able to override a property of a class we import and don't own.

extension UITabBarController {
  open override var shouldAutorotate: Bool {
    return true
  }
}

However due to Objective-C interop the swift compiler does not complain about this extension and let us create an undefined behavior at runtime.

If the name of a method declared in a category is the same as a method in the original class, or a method in another category on the same class (or even a superclass), the behavior is undefined as to which method implementation is used at runtime. This is less likely to be an issue if you’re using categories with your own classes, but can cause problems when using categories to add methods to standard Cocoa or Cocoa Touch classes.

Source: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/CustomizingExistingClasses/CustomizingExistingClasses.html

@beccadax
Copy link
Contributor

@beccadax beccadax commented Dec 6, 2019

@swift-ci create

@CodaFi
Copy link
Member

@CodaFi CodaFi commented Dec 18, 2019

Given source compatibility constraints, the best we can do is warn. This is sorta a StarterBug for the Decl Checker. Ping me here for more details.

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Dec 19, 2019

Comment by Faiçal (JIRA)

Hi @CodaFi! Happy to take this on. Would be grateful for any pointer you could give me to get started 🙂.

@CodaFi
Copy link
Member

@CodaFi CodaFi commented Dec 23, 2019

faical (JIRA User) There's a big condition soup in OverrideMatcher::checkOverride that needs to be updated. Detecting whether a decl has been ClangImport'ed into Swift is simple (just ask Decl::hasClangNode). So the logic is roughly: If the type matches, and the decl context for the declaration is an extension, and the extended decl context is ClangImport'ed, then warn. You'll need to do this for subscripts, variables, and functions - you can probably catch-all by just doing it for ValueDecls and using the descriptive decl kind to render a nice diagnostic.

Finally, you'll need some kind of syntax to silence this warning. There's precedent in allowing bogus overrides as long as the user spells them with parens around the type, so we can probably just suggest that in a note carrying a fixit. That is, this ought to warn:

extension UITabBarController {
  open override var shouldAutorotate: Bool { // warning: extension declares override of Objective-C property 'shouldAutorotate'; this will cause unstable runtime behavior
                                                                           // note: add parentheses to silence this warning
    return true
  }
}

This shouldn't

extension UITabBarController {
  open override var shouldAutorotate: (Bool) {
    return true
  }
}

@CodaFi
Copy link
Member

@CodaFi CodaFi commented Dec 23, 2019

@DevAndArtist If you'll forgive a little language-lawyering, technically this behavior is not undefined - the documentation is a little extreme here. Load-time ordering is consistent across runs of an application, and the runtime guarantees that somebody's category or class is going to win. The unstable bit is who that actually is at runtime.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants