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-120] Objective-C non-null can be bypassed. #42742

Open
swift-ci opened this issue Dec 7, 2015 · 10 comments
Open

[SR-120] Objective-C non-null can be bypassed. #42742

swift-ci opened this issue Dec 7, 2015 · 10 comments

Comments

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Dec 7, 2015

Previous ID SR-120
Radar None
Original Reporter bnut (JIRA User)
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee None
Priority Medium

md5: 36e04bbfbd87bfb5e14e8fd9005a976e

relates to:

  • SR-8622 Nonnull Objective-C property that falsely returns nil causes inconsistent Swift behavior

Issue Description:

My team finds that we are doing a lot of assertions in methods called from Objective-C that are marked as nonnull because you can still send them nil (from Objective-C). Adding these assertions is not possible in Swift. This report outlines the conditions and a potential solution.

Code to reproduce (Swift 2.1):

func doSomething(thing: MyClass) {
   ...
}
-(void) thisWillFail {
    id thing = nil;
    [instance doSomething: thing];
    [instance doSomethingElse: thing];
}
...
-(void) doSomethingElse: (nonnull MyClass *) thing {
   NSParameterAssert(thing);
   ...
}

This NSParameterAssert can be done in ObjC implementations, but because it's not even an implicitly unwrapped optional that does not work in Swift.

Proposed Solution:

It would be nice to have an assertion implicitly inserted at the caller in Objective-C when the value isn't already annotated as non-null (perhaps: strong reference, argument or object returned from method marked as non-null (compiled with the correct version of swift?)).

Likewise it is possible to cast a Swift type to an unrelated Swift type in Objective-C and pass it back to Swift, perhaps this nullability assertion could be extended to also verify that class or protocol conformance is also correct.

Side-note:

Note, MyClass was defined as follows (using NSString had an empty string rather than a nullptr).

@interface MyClass : NSObject
-(instancetype) init UNAVAILABLE_ATTRIBUTE;
-(instancetype) initWithWhatever:(id) whatever;
@end
@belkadan
Copy link
Contributor

@belkadan belkadan commented Dec 8, 2015

If we can do this efficiently this sounds like a great idea. Even if we can't do it efficiently it might be a good idea in Debug builds.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Dec 9, 2015

Comment by Andrew Bennett (JIRA)

Thanks for taking a look Jordan!

@phausler
Copy link
Member

@phausler phausler commented Dec 28, 2015

one note on this; the Darwin version of Foundation actually sometimes uses null as a placeholder sentry value in initializers purposefully to implement certain functionality that is contrary to the declaration of non null in the interface to serve as a internal behavior. If this were added we would need a way to either opt in or opt out of this behavior.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Jan 4, 2016

Philippe, I don't think we ever want to support that in Swift. Instead, you'd just make two entry points, one of them public and one of them internal (and thus testable).

@phausler
Copy link
Member

@phausler phausler commented Jan 4, 2016

how could one make a public and internal exposition in objc?
e.g. -[NSLocale init] calling -[NSLocale initWithLocaleIdentifier:nil] even though initWithLocaleIdentifier: naturally is nonnull

@belkadan
Copy link
Contributor

@belkadan belkadan commented Jan 4, 2016

This would be a better pattern:

public init() {
  self.init(localeIdentifierImpl: nil)
}

public init(localeIdentifier: String) {
  self.init(localeIdentifierImpl: localeIdentifier)
}

internal init(localeIdentifierImpl: String?) {
  // Actual work here.
}

@phausler
Copy link
Member

@phausler phausler commented Jan 4, 2016

again how do I write that in objective-c?

- (id)init { return [self initWithLocaleIdentifier: nil]; }
- (id)initWithLocaleIdentifier:(NSString *)ident { ... }

If the compiler inserts the assertion that would make a function that would previously not crash and return nil now crash if I am understanding the request correctly.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Jan 4, 2016

This particular request is that the synthesized Objective-C entry points for Swift code include nil checks. It wouldn't change anything actually written in Objective-C.

@phausler
Copy link
Member

@phausler phausler commented Jan 4, 2016

I guess I misread it then; disregard my concern then. Your suggestion would be by far preferable in a swift world - most of these cases are from factory patterns anyhow.

@swift-ci
Copy link
Collaborator Author

@swift-ci swift-ci commented Jun 28, 2019

Comment by Andrew Bennett (JIRA)

This is related to a newly discovered issue https://bugs.swift.org/browse/SR-11040

@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