Merge pull request #32300 from aschwaighofer/dyn_repl_objc_method_generic_class_5.3

[5.3] Mark non-foreign entry points of `@objc dynamic` methods in generic classes `dynamically_replaceable`

Author: aschwaighofer

include/swift/AST/Decl.h

@@ -2711,6 +2711,7 @@ class ValueDecl : public Decl {
   /// Is this declaration marked with 'dynamic'?
   bool isDynamic() const;
 
+private:
   bool isObjCDynamic() const {
     return isObjC() && isDynamic();
   }
@@ -2719,6 +2720,37 @@ class ValueDecl : public Decl {
     return !isObjC() && isDynamic();
   }
 
+  bool isObjCDynamicInGenericClass() const;
+
+public:
+  /// Should we use Objective-C method dispatch for this decl.
+  bool shouldUseObjCDispatch() const {
+    return isObjCDynamic();
+  }
+
+  /// Should we use native dynamic function replacement dispatch for this decl.
+  bool shouldUseNativeDynamicDispatch() const {
+    return isNativeDynamic();
+  }
+
+  /// Should we use Objective-C category based function replacement for this
+  /// decl.
+  /// This is all `@objc dynamic` methods except for such methods in native
+  /// generic classes. We can't use a category for generic classes so we use
+  /// native replacement instead (this behavior is only enabled with
+  /// -enable-implicit-dynamic).
+  bool shouldUseObjCMethodReplacement() const;
+
+  /// Should we use native dynamic function replacement mechanism for this decl.
+  /// This is all native dynamic methods except for `@objc dynamic` methods in
+  /// generic classes (see above).
+  bool shouldUseNativeMethodReplacement() const;
+
+  /// Is this a native dynamic function replacement based replacement.
+  /// This is all @_dynamicReplacement(for:) of native functions and @objc
+  /// dynamic methods on generic classes (see above).
+  bool isNativeMethodReplacement() const;
+
   bool isEffectiveLinkageMoreVisibleThan(ValueDecl *other) const {
     return (std::min(getEffectiveAccess(), AccessLevel::Public) >
             std::min(other->getEffectiveAccess(), AccessLevel::Public));

include/swift/Serialization/Validation.h

@@ -98,6 +98,7 @@ class ExtendedValidationInfo {
     unsigned IsSIB : 1;
     unsigned IsTestable : 1;
     unsigned ResilienceStrategy : 2;
+    unsigned IsImplicitDynamicEnabled: 1;
   } Bits;
 public:
   ExtendedValidationInfo() : Bits() {}
@@ -123,6 +124,10 @@ class ExtendedValidationInfo {
   void setPrivateImportsEnabled(bool enabled) {
     Bits.ArePrivateImportsEnabled = enabled;
   }
+  bool isImplicitDynamicEnabled() { return Bits.IsImplicitDynamicEnabled; }
+  void setImplicitDynamicEnabled(bool val) {
+    Bits.IsImplicitDynamicEnabled = val;
+  }
   bool isTestable() const { return Bits.IsTestable; }
   void setIsTestable(bool val) {
     Bits.IsTestable = val;

lib/AST/ASTVerifier.cpp

@@ -3111,12 +3111,12 @@ class Verifier : public ASTWalker {
                storageDecl->getWriteImpl() ==
                    WriteImplKind::StoredWithObservers ||
                storageDecl->getWriteImpl() == WriteImplKind::MutableAddress) &&
-              storageDecl->isNativeDynamic()) &&
+              storageDecl->shouldUseNativeDynamicDispatch()) &&
             // We allow a non dynamic getter if there is a dynamic read.
             !(FD->isGetter() &&
               (storageDecl->getReadImpl() == ReadImplKind::Read ||
                storageDecl->getReadImpl() == ReadImplKind::Address) &&
-              storageDecl->isNativeDynamic())) {
+              storageDecl->shouldUseNativeDynamicDispatch())) {
           Out << "Property and accessor do not match for 'dynamic'\n";
           abort();
         }

lib/AST/Decl.cpp

@@ -1950,7 +1950,7 @@ SourceRange IfConfigDecl::getSourceRange() const {
 }
 
 static bool isPolymorphic(const AbstractStorageDecl *storage) {
-  if (storage->isObjCDynamic())
+  if (storage->shouldUseObjCDispatch())
     return true;
 
 
@@ -2083,7 +2083,7 @@ getDirectReadWriteAccessStrategy(const AbstractStorageDecl *storage) {
     return AccessStrategy::getStorage();
   case ReadWriteImplKind::Stored: {
     // If the storage isDynamic (and not @objc) use the accessors.
-    if (storage->isNativeDynamic())
+    if (storage->shouldUseNativeDynamicDispatch())
       return AccessStrategy::getMaterializeToTemporary(
           getOpaqueReadAccessStrategy(storage, false),
           getOpaqueWriteAccessStrategy(storage, false));
@@ -2168,7 +2168,7 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics,
       if (isPolymorphic(this))
         return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ true);
 
-      if (isNativeDynamic())
+      if (shouldUseNativeDynamicDispatch())
         return getOpaqueAccessStrategy(this, accessKind, /*dispatch*/ false);
 
       // If the storage is resilient from the given module and resilience
@@ -2926,6 +2926,59 @@ bool ValueDecl::isDynamic() const {
     getAttrs().hasAttribute<DynamicAttr>());
 }
 
+bool ValueDecl::isObjCDynamicInGenericClass() const {
+  if (!isObjCDynamic())
+    return false;
+
+  auto *DC = this->getDeclContext();
+  auto *classDecl = DC->getSelfClassDecl();
+  if (!classDecl)
+    return false;
+
+  return classDecl->isGenericContext() && !classDecl->usesObjCGenericsModel();
+}
+
+bool ValueDecl::shouldUseObjCMethodReplacement() const {
+  if (isNativeDynamic())
+    return false;
+
+  if (getModuleContext()->isImplicitDynamicEnabled() &&
+      isObjCDynamicInGenericClass())
+    return false;
+
+  return isObjCDynamic();
+}
+
+bool ValueDecl::shouldUseNativeMethodReplacement() const {
+  if (isNativeDynamic())
+    return true;
+
+  if (!isObjCDynamicInGenericClass())
+    return false;
+
+  auto *replacedDecl = getDynamicallyReplacedDecl();
+  if (replacedDecl)
+    return false;
+
+  return getModuleContext()->isImplicitDynamicEnabled();
+}
+
+bool ValueDecl::isNativeMethodReplacement() const {
+  // Is this a @_dynamicReplacement(for:) that use the native dynamic function
+  // replacement mechanism.
+  auto *replacedDecl = getDynamicallyReplacedDecl();
+  if (!replacedDecl)
+    return false;
+
+  if (isNativeDynamic())
+    return true;
+
+  if (isObjCDynamicInGenericClass())
+    return replacedDecl->getModuleContext()->isImplicitDynamicEnabled();
+
+  return false;
+}
+
 void ValueDecl::setIsDynamic(bool value) {
   assert(!LazySemanticInfo.isDynamicComputed ||
          LazySemanticInfo.isDynamic == value);
@@ -5143,7 +5196,7 @@ bool AbstractStorageDecl::hasDidSetOrWillSetDynamicReplacement() const {
 
 bool AbstractStorageDecl::hasAnyNativeDynamicAccessors() const {
   for (auto accessor : getAllAccessors()) {
-    if (accessor->isNativeDynamic())
+    if (accessor->shouldUseNativeDynamicDispatch())
       return true;
   }
   return false;

lib/IRGen/GenArchetype.cpp

@@ -468,7 +468,7 @@ bool shouldUseOpaqueTypeDescriptorAccessor(OpaqueTypeDecl *opaque) {
 
   // Don't emit accessors for functions that are not dynamic or dynamic
   // replacements.
-  return namingDecl->isNativeDynamic() ||
+  return namingDecl->shouldUseNativeDynamicDispatch() ||
          (bool)namingDecl->getDynamicallyReplacedDecl();
 }
 

lib/IRGen/GenDecl.cpp

@@ -2539,7 +2539,7 @@ void IRGenModule::emitOpaqueTypeDescriptorAccessor(OpaqueTypeDecl *opaque) {
   // Don't emit accessors for functions that are not dynamic or dynamic
   // replacements.
   if (!abstractStorage) {
-    isNativeDynamic = namingDecl->isNativeDynamic();
+    isNativeDynamic = namingDecl->shouldUseNativeDynamicDispatch();
     if (!isNativeDynamic && !isDynamicReplacement)
       return;
   }

lib/IRGen/GenMeta.cpp

@@ -1551,7 +1551,7 @@ namespace {
       auto flags = getMethodDescriptorFlags<Flags>(func);
 
       // Remember if the declaration was dynamic.
-      if (func->isObjCDynamic())
+      if (func->shouldUseObjCDispatch())
         flags = flags.withIsDynamic(true);
 
       // Include the pointer-auth discriminator.

lib/IRGen/GenObjC.cpp

@@ -1430,12 +1430,22 @@ void irgen::emitObjCSetterDescriptor(IRGenModule &IGM,
   emitObjCDescriptor(IGM, descriptors, descriptor);
 }
 
+static bool isObjCGenericClassExtension(ValueDecl *decl) {
+  // Don't emit category entries for @objc methods in extensions they would
+  // normally be disallowed except for @_dynamicReplacement(for:) methods that
+  // use the native dynamic replacement mechanism instead of objc categories.
+  auto *DC = decl->getDeclContext();
+  if (!isa<ExtensionDecl>(DC))
+    return false;
+  return decl->isNativeMethodReplacement();
+}
+
 bool irgen::requiresObjCMethodDescriptor(FuncDecl *method) {
   // Property accessors should be generated alongside the property.
   if (isa<AccessorDecl>(method))
     return false;
 
-  return method->isObjC();
+  return method->isObjC() && !isObjCGenericClassExtension(method);
 }
 
 bool irgen::requiresObjCMethodDescriptor(ConstructorDecl *constructor) {
@@ -1447,12 +1457,13 @@ bool irgen::requiresObjCPropertyDescriptor(IRGenModule &IGM,
   // Don't generate a descriptor for a property without any accessors.
   // This is only possible in SIL files because Sema will normally
   // implicitly synthesize accessors for @objc properties.
-  return property->isObjC() && property->requiresOpaqueAccessors();
+  return property->isObjC() && property->requiresOpaqueAccessors() &&
+         !isObjCGenericClassExtension(property);
 }
 
 bool irgen::requiresObjCSubscriptDescriptor(IRGenModule &IGM,
                                             SubscriptDecl *subscript) {
-  return subscript->isObjC();
+  return subscript->isObjC() && !isObjCGenericClassExtension(subscript);
 }
 
 llvm::Value *IRGenFunction::emitBlockCopyCall(llvm::Value *value) {

lib/SIL/IR/SILDeclRef.cpp

@@ -41,7 +41,7 @@ swift::getMethodDispatch(AbstractFunctionDecl *method) {
   auto dc = method->getDeclContext();
 
   if (dc->getSelfClassDecl()) {
-    if (method->isObjCDynamic()) {
+    if (method->shouldUseObjCDispatch()) {
       return MethodDispatch::Class;
     }
 
@@ -88,7 +88,7 @@ bool swift::requiresForeignToNativeThunk(ValueDecl *vd) {
 bool swift::requiresForeignEntryPoint(ValueDecl *vd) {
   assert(!isa<AbstractStorageDecl>(vd));
 
-  if (vd->isObjCDynamic()) {
+  if (vd->shouldUseObjCDispatch()) {
     return true;
   }
 
@@ -867,15 +867,15 @@ SILDeclRef SILDeclRef::getNextOverriddenVTableEntry() const {
     }
 
     // Overrides of @objc dynamic declarations are not in the vtable.
-    if (overridden.getDecl()->isObjCDynamic()) {
+    if (overridden.getDecl()->shouldUseObjCDispatch()) {
       return SILDeclRef();
     }
-    
+
     if (auto *accessor = dyn_cast<AccessorDecl>(overridden.getDecl())) {
       auto *asd = accessor->getStorage();
       if (asd->hasClangNode())
         return SILDeclRef();
-      if (asd->isObjCDynamic()) {
+      if (asd->shouldUseObjCDispatch()) {
         return SILDeclRef();
       }
     }
@@ -1106,6 +1106,10 @@ static bool isDesignatedConstructorForClass(ValueDecl *decl) {
 }
 
 bool SILDeclRef::canBeDynamicReplacement() const {
+  // The foreign entry of a @dynamicReplacement(for:) of @objc method in a
+  // generic class can't be a dynamic replacement.
+  if (isForeign && hasDecl() && getDecl()->isNativeMethodReplacement())
+    return false;
   if (kind == SILDeclRef::Kind::Destroyer ||
       kind == SILDeclRef::Kind::DefaultArgGenerator)
     return false;
@@ -1117,6 +1121,11 @@ bool SILDeclRef::canBeDynamicReplacement() const {
 }
 
 bool SILDeclRef::isDynamicallyReplaceable() const {
+  // The non-foreign entry of a @dynamicReplacement(for:) of @objc method in a
+  // generic class can't be a dynamically replaced.
+  if (!isForeign && hasDecl() && getDecl()->isNativeMethodReplacement())
+    return false;
+
   if (kind == SILDeclRef::Kind::DefaultArgGenerator)
     return false;
   if (isStoredPropertyInitializer() || isPropertyWrapperBackingInitializer())
@@ -1138,5 +1147,15 @@ bool SILDeclRef::isDynamicallyReplaceable() const {
     return false;
 
   auto decl = getDecl();
-  return decl->isNativeDynamic();
+
+  if (isForeign)
+    return false;
+
+  // We can't generate categories for generic classes. So the standard mechanism
+  // for replacing @objc dynamic methods in generic classes does not work.
+  // Instead we mark the non @objc entry dynamically replaceable and replace
+  // that.
+  // For now, we only support this behavior if -enable-implicit-dynamic is
+  // enabled.
+  return decl->shouldUseNativeMethodReplacement();
 }

lib/SIL/IR/SILFunctionBuilder.cpp

@@ -91,7 +91,10 @@ void SILFunctionBuilder::addFunctionAttributes(
   auto *decl = constant.getDecl();
 
   // Only emit replacements for the objc entry point of objc methods.
-  if (decl->isObjC() &&
+  // There is one exception: @_dynamicReplacement(for:) of @objc methods in
+  // generic classes. In this special case we use native replacement instead of
+  // @objc categories.
+  if (decl->isObjC() && !decl->isNativeMethodReplacement() &&
       F->getLoweredFunctionType()->getExtInfo().getRepresentation() !=
           SILFunctionTypeRepresentation::ObjCMethod)
     return;
@@ -103,7 +106,10 @@ void SILFunctionBuilder::addFunctionAttributes(
   if (!replacedDecl)
     return;
 
-  if (decl->isObjC()) {
+  // For @objc method replacement we normally use categories to perform the
+  // replacement. Except for methods in generic class where we can't. Instead,
+  // we special case this and use the native swift replacement mechanism.
+  if (decl->isObjC() && !decl->isNativeMethodReplacement()) {
     F->setObjCReplacement(replacedDecl);
     return;
   }