I'm working on a fix for #82618. For us this is a blocker for incremental Swift 6 adoption, but it also reproduces without concurrency features.
The bug can be fixed in different ways:
Rejecting the code with an compile-time error
Updating code gen to not use _dictionaryUpcast
Making dynamic casting of closures actually work (might not be feasible)
For now I’m going with first option as a short-term solution and then will check if I can implement the second one. But I would appreciate more input about the expected behavior here.
3 is not feasible, but for 2, you could add a new entry point that takes a closure to perform the conversion of each element, instead of relying on dynamic casts. The new entry point would then be used for dictionary conversions where the destination type is statically known. Probably array conversions suffer from the same problem too.
I've found out that CollectionUpcastConversionExpr contains KeyConversion and ValueConversion, which should be sufficient to generate correct code in SILGen. And I guess they kinda were supposed to be used for that.
But I'm still quite lost in the guts of the constraint solver, and currently I'm not able to distinguish cases where dictionaryUpcast would work vs cases where it would not. So I cannot only implement a fix where new entry point would be used always. It is good enough, or would it have performance implications?
It might be better to reject the unsupported conversions in CSApply instead. The solution is still valid, it just can’t be accepted because of a (hopefully temporary) limitation.
I'm exploring the idea of checking content of the KeyConversion/ValueConversion to check if dictionaryUpcast and similar old functions are applicable.
I wanted to start with something simpler, so I've tried to deal with arrays first, so I've started looking at the _arrayForceCast function. But this logic seems to be too permissive. E.g. this cast succeeds, and instead code crashes when trying to call the block:
let strings = ["abc" as NSString, "xyz" as NSString]
let blocks = strings as! [@convention(block) () -> Void]
for b in blocks {
b() // crash
}
Is this a yet another bug, or is this expected?
Note that conditional cast as? fails as expected, because it uses _arrayConditionalCast which performs O(n) check of each element.
This is a bug. The cast should not succeed in this case, unless there is some secret NSString to block conversion (where it JITs some Objective-C code for you? :) ) inside the objc runtime.
In general I would expect that (x as? T)! produce the same result as x as! T, but I’m sure there are other exceptions.
But when the last case is passed to swift_bridgeObjectRetain(), then the lowest bit is unexpectedly cleared:
public func test() {
let arr = ["x" as NSString, "y" as NSString]
// native.storesOnlyElementsOfType(TargetElement.self) returns false
// constructs array with deferred type checking
let w = arr as! [@convention(block) () -> Void]
foo(w)
}
func foo(_ x : [@convention(block) () -> Void]) {
// let b0 = x[0] // triggers assertion, lowest bit is set, _isNativeTypeChecked returns true
// b0()
// produces result with lowest bit cleared inside swift_bridgeObjectRetain()
var iter = x.makeIterator()
// does not trigger assertion, lowest bit is cleared, _isNativeTypeChecked returns false
while let b = iter.next() {
b()
}
}
When simply copying arrays, I cannot reproduce this. swift_bridgeObjectRetain() gets called, but its result is discarded. Instead value of it's argument is used as a raw value of the copy:
%15 = call ptr @swift_bridgeObjectRetain(ptr returned %14) #2
store ptr %14, ptr %copy.debug, align 8
I see that IR signature of the swift_bridgeObjectRetain contains returned attribute. Current implementation of the swift_bridgeObjectRetain() violates this. Do I understand correctly that this needs to be fixed inside swift_bridgeObjectRetain() and related functions?
Moving forward, it seems to be possible to make a decision based on the contents of the KeyConversion/ValueConversion and if needed, to convert them into closures.
Current code uses SILGenFunction::emitCollectionConversion() and SILGenFunction::emitApplyOfLibraryIntrinsic() which don't have all the functionality involved in SILGenFunction::emitApply() and there is no correct FunctionTypeInfo for the synthesised closure.
I can continue moving in the current direction, and manually create FunctionTypeInfo (pretty much hardcoded). Alternatively, I could synthesise complete ApplyExpr replacing entire CollectionUpcastConversionExpr. Maybe even before SILGen phase. What would be the best place for this then?
After more digging, I've found a similar use case for generating SIL for async let which uses emitConvertedRValue(), and using it as a reference, I was able to solve closure emissions.
Now, with this fix it is possible to have this paradoxical situation:
let a: [() -> Derived] = [ ... ]
let b: [() -> Base] = a // ok
expectEqual(b.count, 3)
let c = a as? [() -> Base] // warning: conditional cast from '[() -> Derived]' to '[() -> Base]' always succeeds
expectEqual(c?.count, 3) // fails, c is nil
let d = (a as Any) as? [() -> Base]
expectNil(d) // d is nil
So far, I don't have ideas on how to improve this. So I will submit PR with this behavior.