Merge pull request #14825 from stephan-tolksdorf/SR-6065

[SIL] [SR-6065] Take into account ObjC selector name when determining method family

Author: jrose-apple

lib/IRGen/GenObjC.cpp

@@ -487,21 +487,6 @@ namespace {
 
     static constexpr struct ForGetter_t { } ForGetter{};
     static constexpr struct ForSetter_t { } ForSetter{};
-
-#define FOREACH_FAMILY(FAMILY)         \
-    FAMILY(Alloc, "alloc")             \
-    FAMILY(Copy, "copy")               \
-    FAMILY(Init, "init")               \
-    FAMILY(MutableCopy, "mutableCopy") \
-    FAMILY(New, "new")
-
-    // Note that these are in parallel with 'prefixes', below.
-    enum class Family {
-      None,
-#define GET_LABEL(LABEL, PREFIX) LABEL,
-      FOREACH_FAMILY(GET_LABEL)
-#undef GET_LABEL
-    };
 
     Selector() = default;
 
@@ -571,31 +556,6 @@ namespace {
     StringRef str() const {
       return Text;
     }
-
-    /// Return the family string of this selector.
-    Family getFamily() const {
-      StringRef text = str();
-      while (!text.empty() && text[0] == '_') text = text.substr(1);
-
-#define CHECK_PREFIX(LABEL, PREFIX) \
-      if (hasPrefix(text, PREFIX)) return Family::LABEL;
-      FOREACH_FAMILY(CHECK_PREFIX)
-#undef CHECK_PREFIX
-
-      return Family::None;
-    }
-
-  private:
-    /// Does the given selector start with the given string as a
-    /// prefix, in the sense of the selector naming conventions?
-    static bool hasPrefix(StringRef text, StringRef prefix) {
-      if (!text.startswith(prefix)) return false;
-      if (text.size() == prefix.size()) return true;
-      assert(text.size() > prefix.size());
-      return !clang::isLowercase(text[prefix.size()]);
-    }
-
-#undef FOREACH_FAMILY
   };
 } // end anonymous namespace
 

lib/SIL/SILFunctionType.cpp

@@ -23,6 +23,7 @@
 #include "swift/AST/ForeignInfo.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/ProtocolConformance.h"
+#include "swift/Basic/StringExtras.h"
 #include "swift/SIL/SILModule.h"
 #include "swift/SIL/SILType.h"
 #include "clang/AST/Attr.h"
@@ -1723,19 +1724,12 @@ static SelectorFamily getSelectorFamily(Identifier name) {
   StringRef text = name.get();
   while (!text.empty() && text[0] == '_') text = text.substr(1);
 
-  /// Does the given selector start with the given string as a
-  /// prefix, in the sense of the selector naming conventions?
-  auto hasPrefix = [](StringRef text, StringRef prefix) {
-    if (!text.startswith(prefix)) return false;
-    if (text.size() == prefix.size()) return true;
-    assert(text.size() > prefix.size());
-    return !clang::isLowercase(text[prefix.size()]);
-  };
+  StringRef firstWord = camel_case::getFirstWord(text);
 
   auto result = SelectorFamily::None;
   if (false) /*for #define purposes*/;
 #define CHECK_PREFIX(LABEL, PREFIX) \
-  else if (hasPrefix(text, PREFIX)) result = SelectorFamily::LABEL;
+  else if (firstWord == PREFIX) result = SelectorFamily::LABEL;
   FOREACH_FAMILY(CHECK_PREFIX)
 #undef CHECK_PREFIX
 
@@ -1744,61 +1738,49 @@ static SelectorFamily getSelectorFamily(Identifier name) {
   return result;
 }
 
-/// Get the ObjC selector family a SILDeclRef implicitly belongs to.
+/// Get the ObjC selector family a foreign SILDeclRef belongs to.
 static SelectorFamily getSelectorFamily(SILDeclRef c) {
+  assert(c.isForeign);
   switch (c.kind) {
   case SILDeclRef::Kind::Func: {
     if (!c.hasDecl())
       return SelectorFamily::None;
 
     auto *FD = cast<FuncDecl>(c.getDecl());
-    auto accessor = dyn_cast<AccessorDecl>(FD);
-    if (!accessor)
-      return getSelectorFamily(FD->getName());
-
-    switch (accessor->getAccessorKind()) {
-    case AccessorKind::IsGetter:
-      // Getter selectors can belong to families if their name begins with the
-      // wrong thing.
-      if (accessor->getStorage()->isObjC() || c.isForeign) {
-        auto declName = accessor->getStorage()->getBaseName();
-        switch (declName.getKind()) {
-        case DeclBaseName::Kind::Normal:
-          return getSelectorFamily(declName.getIdentifier());
-        case DeclBaseName::Kind::Subscript:
-          return SelectorFamily::None;
-        case DeclBaseName::Kind::Destructor:
-          return SelectorFamily::None;
-        }
+    if (auto accessor = dyn_cast<AccessorDecl>(FD)) {
+      switch (accessor->getAccessorKind()) {
+      case AccessorKind::IsGetter:
+      case AccessorKind::IsSetter:
+        break;
+      case AccessorKind::IsWillSet:
+      case AccessorKind::IsDidSet:
+      case AccessorKind::IsAddressor:
+      case AccessorKind::IsMutableAddressor:
+      case AccessorKind::IsMaterializeForSet:
+        llvm_unreachable("Unexpected AccessorKind of foreign FuncDecl");
       }
-      return SelectorFamily::None;
-
-      // Other accessors are never selector family members.
-    case AccessorKind::IsSetter:
-    case AccessorKind::IsWillSet:
-    case AccessorKind::IsDidSet:
-    case AccessorKind::IsAddressor:
-    case AccessorKind::IsMutableAddressor:
-    case AccessorKind::IsMaterializeForSet:
-      return SelectorFamily::None;
     }
+
+    return getSelectorFamily(FD->getObjCSelector().getSelectorPieces().front());
   }
   case SILDeclRef::Kind::Initializer:
-    case SILDeclRef::Kind::IVarInitializer:
+  case SILDeclRef::Kind::IVarInitializer:
     return SelectorFamily::Init;
 
   /// Currently IRGen wraps alloc/init methods into Swift constructors
   /// with Swift conventions.
   case SILDeclRef::Kind::Allocator:
   /// These constants don't correspond to method families we care about yet.
-  case SILDeclRef::Kind::EnumElement:
   case SILDeclRef::Kind::Destroyer:
   case SILDeclRef::Kind::Deallocator:
-  case SILDeclRef::Kind::GlobalAccessor:
   case SILDeclRef::Kind::IVarDestroyer:
+    return SelectorFamily::None;
+
+  case SILDeclRef::Kind::EnumElement:
+  case SILDeclRef::Kind::GlobalAccessor:
   case SILDeclRef::Kind::DefaultArgGenerator:
   case SILDeclRef::Kind::StoredPropertyInitializer:
-    return SelectorFamily::None;
+    llvm_unreachable("Unexpected Kind of foreign SILDeclRef");
   }
 
   llvm_unreachable("Unhandled SILDeclRefKind in switch.");

test/PrintAsObjC/classes.swift

@@ -226,6 +226,7 @@ class NotObjC {}
 // CHECK-NEXT: - (void)initAllTheThings SWIFT_METHOD_FAMILY(none);
 // CHECK-NEXT: - (void)initTheOtherThings SWIFT_METHOD_FAMILY(none);
 // CHECK-NEXT: - (void)initializeEvenMoreThings;
+// CHECK-NEXT: + (Methods * _Nonnull)newWithFoo:(NSInteger)foo SWIFT_WARN_UNUSED_RESULT;
 // CHECK-NEXT: init
 // CHECK-NEXT: @end
 @objc class Methods {
@@ -286,6 +287,8 @@ class NotObjC {}
   @objc func initAllTheThings() {}
   @objc(initTheOtherThings) func setUpOtherThings() {}
   @objc func initializeEvenMoreThings() {}
+
+  @objc(newWithFoo:) class func make(foo: Int) -> Methods { return Methods() }
 }
 
 typealias AliasForNSRect = NSRect

test/SILGen/objc_thunks.swift

@@ -57,6 +57,20 @@ class Hoozit : Gizmo {
   // CHECK-NEXT:   return [[RES]]
   // CHECK-NEXT: }
 
+  // NS_RETURNS_RETAINED by family (-copy)
+  @objc(copyDuplicate) func makeDuplicate() -> Gizmo { return self }
+  // CHECK-LABEL: sil hidden [thunk] @$S11objc_thunks6HoozitC13makeDuplicateSo5GizmoCyFTo : $@convention(objc_method) (Hoozit) -> @owned Gizmo
+  // CHECK: bb0([[THIS:%.*]] : @unowned $Hoozit):
+  // CHECK-NEXT:   [[THIS_COPY:%.*]] = copy_value [[THIS]]
+  // CHECK-NEXT:   [[BORROWED_THIS_COPY:%.*]] = begin_borrow [[THIS_COPY]]
+  // CHECK-NEXT:   // function_ref
+  // CHECK-NEXT:   [[NATIVE:%.*]] = function_ref @$S11objc_thunks6HoozitC13makeDuplicateSo5GizmoCyF : $@convention(method) (@guaranteed Hoozit) -> @owned Gizmo
+  // CHECK-NEXT:   [[RES:%.*]] = apply [[NATIVE]]([[BORROWED_THIS_COPY]])
+  // CHECK-NEXT:   end_borrow [[BORROWED_THIS_COPY]] from [[THIS_COPY]]
+  // CHECK-NEXT:   destroy_value [[THIS_COPY]]
+  // CHECK-NEXT:   return [[RES]]
+  // CHECK-NEXT: }
+
   // Override the normal family conventions to make this non-consuming and
   // returning at +0.
   @objc func initFoo() -> Gizmo { return self }