[Runtime] Add ObjC support to isKnownUniquelyReferenced.

[al45tair]

Add code to support detecting uniquely referenced Objective-C and Core Foundation objects.

rdar://47902425
rdar://66805490

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please clean smoke test

[al45tair]

@swift-ci Please clean smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test macOS platform

[al45tair]

@swift-ci Please smoke test macOS platform

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[beccadax]

A quick glance at the "files changed" view here suggests your indentation settings aren't correct. Perhaps you've set your tabs to be two spaces wide, but you're still inserting tab characters instead of space characters.

[al45tair]

@beccadax Hmmm. I think my editor has picked up the indentation and changed the tab size to match. How annoying. I'll tinker with it. git-clang-format (which I just ran) should hopefully fix that, however.

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please smoke test

[mikeash]

LGTM. Would be good to get a signoff from someone more familiar with IRGen for that part.

[aschwaighofer]

The IRGen changes LGTM.

I have not audited the standard libraries uses of isUnique to make sure no assumptions are made about the buffer if it returns true i.e it might have checked whether the bridged object is unique and then if that is true assumed that the bridged object pointer points to a native object. We probably always track that separately through the is bridged bit in the pointer to the BridgedObject (_BridgeStorage.isNative) but I thought to point this out this might be a risk here.

[al45tair]

I have not audited the standard libraries uses of isUnique to make sure no assumptions are made about the buffer if it returns true i.e it might have checked whether the bridged object is unique and then if that is true assumed that the bridged object pointer points to a native object. We probably always track that separately through the is bridged bit in the pointer to the BridgedObject (_BridgeStorage.isNative) but I thought to point this out this might be a risk here.

It's a good point. I had a look through and I don't think there's any problem with this (and all the tests pass too, which is a good sign). Do we have a standard library expert we can ask, just to confirm?

[aschwaighofer]

I think we should audit that all uses are guarded by _BridgeStorage.isNative or the like when necessary. Which it sounds like you have done. @lorentey Do you have any concerns or know someone who might have?

[aschwaighofer]

I wonder if there is a performance benefit now from changing the two conditions around in ArrayBuffer.swift:

  @inlinable                                                                    
  internal mutating func isUniquelyReferenced() -> Bool {                       
    if !_isClassOrObjCExistential(Element.self) {                               
      return _storage.isUniquelyReferencedUnflaggedNative()                     
    }                                                                           
    return _storage.isUniquelyReferencedNative() && _isNative  // <== swap conditions?               
   }  

_storage.isUniquelyReferencedNative calls isUnique which will go into the objc runtime to check for a unique count (right?) but we really don't care because of the second condition _isNative (which is just a bit we check on the buffer pointer).

[al45tair]

@aschwaighofer Yes, those could be swapped. Also, BridgeStorage's isUniquelyReferencedNative() could maybe be renamed (since it will now return true for uniquely referenced Objective-C types). Not sure whether to do that in this PR, though?

[aschwaighofer]

Follow-up is fine by me.

[al45tair]

Hmmm. Looking at this more carefully, _isNative is true if the Element type is native, not if the buffer is native. That makes me worry about requestUniqueMutableBackingBuffer(), for instance. I need to look at this further and think about it.

[aschwaighofer]

When the Element type is not a class (or objc existential) then we bridge eagerly (convert the element type at bridging time - no objc storage is used).
It only needs asks the buffer storage whether it is native if the element is a class like thing. On the !_isClassOrObjCExistential(Element.self) path the storage cannot be ObjC.

  @inlinable                                                                    
  internal var _isNative: Bool {                                                
    if !_isClassOrObjCExistential(Element.self) {                               
      return true  // <= We know statically this is never bridged so we can return true                                                             
    } else {                                                                    
      return _storage.isNative    <== Dynamically calls BridgeStorage.swift code to check  the storage                                            
    }                                                                           
  } 

//  - `Array<Element>` is like `ContiguousArray<Element>` when `Element` is not 
//    a reference type or an Objective-C existential.  Otherwise, it may use    
//    an `NSArray` bridged from Cocoa for storage.  

[al45tair]

@swift-ci Please smoke test

[al45tair]

@swift-ci Please test