Comment by Andrew Bennett (JIRA)

This might be a duplicate of SR-8622. It was determined in that one by @belkadan that:

The cost of checking every nonnull return value was determined to be too high, but maybe we could do it in Debug builds.

Although it seems like it's already being checked for bridged types, and in this particular case it may not be too expensive to treat it as .none rather than .some(nil!).

Comment by Ding Ye (JIRA)

I am not sure if we can call it a Swift compiler bug. nonnull Methods/properties returning nil can causes undefined behaviour when used, and thus the inconsistency of Debug/Release modes technically makes sense.

The logic of if let x = obj.objcProperty can be broken down into the following two lines of code:

let tmp: SomeSwiftType? = obj.objcProperty  // Line 1
if let x = tmp {  }   // Line 2

In Line 1, an optional (i.e. enum) variable tmp is allocated. Then, it checks whether the getter of property objcProperty exists. If it exists, it injects .some to the enum tmp and maps value of the .some case to the result of the getter; otherwise, it injects .none to the enum tmp. Note that the injection is performed via inject-enum-addr, which yields undefined behaviour when the data for a mismatches case is stored.

In Line 2, it checks which enum case tmp is injected. If it is .some, it assigns the mapped value to x and then goes into the curly braces.

In your specific case, the inject-enum-addr in Line 1 has undefined behaviour, since the data nil mismatches SomeSwiftType for the .some case. Such undefined behaviour may lead to inconsistent observations under different circumstances (e.g., Release/Debug modes).

Anyway, it would be good if clang could raise warnings at the nil returning points in methods declared nonnull.

Comment by Andrew Bennett (JIRA)

Yeah, I don’t mind whether this is fixed at runtime, but because it only happens on release builds and a lot of the code is optimized out it means that it’s very hard to track down why it’s crashing.

We changed it to a required property that is nullable, and at least now ObjC has warnings when you do it wrong (don’t implement it).

The real bug IMO is that the nullability requirements of the ObjC interface are not enforced (or warnings) on the implementation. As you said dingobye (JIRA User). This seems inconsistent and incomplete, and for us lead to a difficult to find bug.

This ticket is probably mislabeled, I’m sorry, but there didn’t seem to be a better way to label it.

Comment by Ding Ye (JIRA)

For nonnull interfaces, we can do some improvements in any (or all) of the following three aspects:

[1] It should have been clearly documented somewhere that violating nonnull can introduce undefined behaviour. It makes sense that users themselves are responsible to keep the returned value nonnull (e.g., checking nullability in the ObjC code before returning)., however the consequence of violation should have been clearly presented.

[2] Improve the clang compiler to raise warnings when nil is found as a return value. However, it can only capture constant nil value, since ObjC is not strongly typed.

[3] Enforce the nullability at runtime. For every nonnull method, the compiler can help to synthesize code of nullability check immediately before returning. However, this introduces some overhead.

I think we should consolidate discussion in SR-8622, since the @optional ends up not figuring in here. These are all good points, though.