[RFC] Restore using __SwiftValue in store on Linux

[zayass]

Currently bridging may crash on as! if you try to cast [T] to NSArray where T is not inherit from of NSObject

Before #1526 such cases covered by boxing into __SwiftValue. This change restores that behavior

[compnerd]

CC: @millenomi

[compnerd]

@swift-ci please test

[millenomi]

Wait, how does this solve the problem?

[millenomi]

Before #1526 you could never cast [T] to a NSArray without an explicit __SwiftValue.store(), so the commit message makes little sense to me.

[lxbndr]

@millenomi I was recently checking a dynamic cast issue (made a test here #2874) and it looks related to code mentioned in this patch.

class TestClass {}
let anyArray: Any = [TestClass()]
let obj = anyArray as? NSObject      // <- crash

The stack trace leads exactly to Bridging.swift:

[0x0]   ucrtbase!abort + 0x4e
[0x1]   swiftCore!swift_reportError + 0xa7
[0x2]   swiftCore!swift_getMangledTypeName + 0x447
[0x3]   swiftCore!swift_getMangledTypeName + 0x4ba
[0x4]   swiftCore!swift_dynamicCastClassUnconditional + 0x8e
[0x5]   Foundation!static Foundation.__SwiftValue.store(Any?) -> Foundation.NSObject? + 0x57e (Bridging.swift:199)
[0x6]   Foundation!(extension in Foundation):Swift.Array._bridgeToObjectiveC() -> Foundation.NSArray + 0xf9 (Array.swift:15)
[0x7]   Foundation!(extension in Foundation):Swift.Array._bridgeToObjectiveC() -> Foundation.NSArray + 0x13a (Array.swift:15)
[0x8]   swiftCore!(extension in Swift):Swift.Collection.map<A>((A.Element) throws -> A1) throws -> [A1] + 0x2d8
[0x9]   Foundation!(extension in Foundation):Swift.Array._bridgeToObjectiveC() -> Foundation.NSArray + 0x93 (Array.swift:14) 
[0xa]   swiftCore!swift_dynamicCast + 0x8e7
[0xb]   swiftCore!swift_dynamicCast + 0x5ee
[0xc]   swiftCore!swift_dynamicCast + 0x12d

The last trace of crash in Foundation is Bridging.swift:199.

Applying this patch fixes the crash in my test, and allows it to pass.

Could you please revisit this? Maybe with additional context it would make more sense.

[millenomi]

I'm very confused because casting [T] to a NSArray for some T should produce a NSArray of T-containing __SwiftValues, and also because you're eliminating the Darwin special handling that was there to prevent us from messing up when we were double-boxing (wrapping a T in a Darwin Foundation NSObject and then trying to access it as a SwiftFoundation NSObject.)

tl;dr: I need to be explained why the patch fixes the problem of NSArray failing to bridge the Ts to __SwiftValue boxes, rather than just avoiding a crash.

[lxbndr]

Just to be sure:

casting [T] to a NSArray for some T should produce a NSArray of T-containing __SwiftValues

is the expected result, right?

What I see is happening in current code:

1. anyArray as? NSObject makes call to Array._bridgeToObjectiveC(). Its implementation is pretty short:

public func _bridgeToObjectiveC() -> _ObjectType {
    return NSArray(array: map { (element: Element) -> AnyObject in
        return __SwiftValue.store(element)
    })
}

I.e. it actually tries to do what we expecting: produce a NSArray of __SwiftValues.

2. The implementation of __SwiftValue.store checks if passed value is NSObject or Unwrappable with nil, and then we get to the code we discussing. For Darwin it does additional check. I guess it is that protection against double-boxing you mentioned:

let boxed = (value as AnyObject)
if !(boxed is NSObject) {
    return __SwiftValue(value) // Do not emit native boxes — wrap them in Swift Foundation boxes instead.
} else {
    return boxed as! NSObject
}

For non-Darwin there is no additional checks, and we are just doing forced cast:

return (value as AnyObject) as! NSObject

This cast crashes when value is not NSObject.

What this patch suggests, is to make Darwin part not only for Darwin, but "unlock" it for all platforms, as there we have explicit wrapping into __SwiftValue. This will produce NSArray of __SwiftValues, i.e. the result we expect.

Sadly, I am not familiar with mechanics of Swift bridging and casting. For me non-Darwin part even not tries to wrap value into __SwiftValue. Maybe I am missing something. Is that crashing line expected to generate __SwiftValue wrapper implicitly? And the issue is in compiler then. I appreciate any help with correct fix of this issue.

[millenomi]

Ah, whoops — turns out you're correct on all counts, and I just didn't make the connection there. You are also correct to note that a non-NSObject result of (x as AnyObject) can and does exist.

Let's just fix your conflicts and take this patch; it is correct.

[millenomi]

@swift-ci please test and merge

[millenomi]

@swift-ci please test and merge