Clang importer: use importFullName for factory methods as initializers.

DougGregor · DougGregor · commit 91d1c3b21a0d · 2015-12-03T11:50:42.000-08:00
Eliminates one of the redundant places where we map Objective-C
selectors over to Swift names.
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -74,8 +74,6 @@ STATISTIC(NumMultiMethodNames,
           "multi-part selector method names imported");
 STATISTIC(NumMethodsMissingFirstArgName,
           "selectors where the first argument name is missing");
-STATISTIC(NumFactoryMethodsNullary,
-          "# of factory methods not mapped due to nullary with long name");
 STATISTIC(NumInitsDroppedWith,
           "# of initializer selectors from which \"with\" was dropped");
 STATISTIC(NumInitsPrepositionSplit,
@@ -1731,9 +1729,11 @@ auto ClangImporter::Implementation::importFullName(
 
       // Set the first argument name to be the name we computed. If
       // there is no first argument, create one for this purpose.
-      if (argumentNames.empty() && !argName.empty()) {
-        // FIXME: Record what happened here for the caller?
-        argumentNames.push_back(argName);
+      if (argumentNames.empty()) {
+        if (!argName.empty()) {
+          // FIXME: Record what happened here for the caller?
+          argumentNames.push_back(argName);
+        }
       } else {
         argumentNames[0] = argName;
       }
@@ -1881,6 +1881,12 @@ auto ClangImporter::Implementation::importFullName(
     if (isInitializer) {
       // For initializers, prepend "__" to the first argument name.
       if (argumentNames.empty()) {
+        // FIXME: ... unless it was from a factory method, for historical
+        // reasons.
+        if (result.InitKind == CtorInitializerKind::Factory ||
+            result.InitKind == CtorInitializerKind::ConvenienceFactory)
+          return result;
+
         // FIXME: Record that we did this.
         argumentNames.push_back("__");
       } else {
@@ -2196,50 +2202,6 @@ ClangImporter::Implementation::mapSelectorToDeclName(ObjCSelector selector,
   return result;
 }
 
-DeclName ClangImporter::Implementation::mapFactorySelectorToInitializerName(
-    ObjCSelector selector,
-    StringRef className,
-    bool isSwiftPrivate) {
-  // See if we can match the class name to the beginning of the first selector
-  // piece.
-  auto firstPiece = selector.getSelectorPieces()[0];
-  StringRef firstArgLabel = matchLeadingTypeName(firstPiece.str(), className);
-  if (firstArgLabel.size() == firstPiece.str().size())
-    return DeclName();
-
-  // Form the first argument label.
-  llvm::SmallString<32> scratch;
-  SmallVector<Identifier, 4> argumentNames;
-  argumentNames.push_back(
-    importArgName(SwiftContext,
-                  camel_case::toLowercaseWord(firstArgLabel, scratch),
-                  /*dropWith=*/true,
-                  isSwiftPrivate));
-
-  // Handle nullary factory methods.
-  if (selector.getNumArgs() == 0) {
-    if (argumentNames[0].empty())
-      return DeclName(SwiftContext, SwiftContext.Id_init, { });
-
-    // We don't have a convenience place to put the remaining argument name,
-    // so leave it as a factory method.
-    ++NumFactoryMethodsNullary;
-    return DeclName();
-  }
-
-  // Map the remaining selector pieces.
-  for (auto piece : selector.getSelectorPieces().slice(1)) {
-    if (piece.empty())
-      argumentNames.push_back(piece);
-    else
-      argumentNames.push_back(importArgName(SwiftContext, piece.str(),
-                                            /*dropWith=*/false,
-                                            /*isSwiftPrivate=*/false));
-  }
-
-  return DeclName(SwiftContext, SwiftContext.Id_init, argumentNames);
-}
-
 /// Translate the "nullability" notion from API notes into an optional type
 /// kind.
 OptionalTypeKind ClangImporter::Implementation::translateNullability(
@@ -2676,6 +2638,11 @@ bool ClangImporter::Implementation::shouldImportAsInitializer(
     if (firstArgLabel.size() == firstPiece.size())
       return false;
 
+    // FIXME: Factory methods cannot have dummy parameters added for
+    // historical reasons.
+    if (!firstArgLabel.empty() && method->getSelector().getNumArgs() == 0)
+      return false;
+
     // Store the prefix length.
     prefixLength = firstPiece.size() - firstArgLabel.size();
 
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -48,8 +48,6 @@
 #define DEBUG_TYPE "Clang module importer"
 
 STATISTIC(NumTotalImportedEntities, "# of imported clang entities");
-STATISTIC(NumFactoryMethodsWrongResult,
-          "# of factory methods not mapped due to an incorrect result type");
 STATISTIC(NumFactoryMethodsAsInitializers,
           "# of factory methods mapped to initializers");
 
@@ -2783,75 +2781,22 @@ namespace {
                                      const clang::ObjCMethodDecl *decl,
                                      ObjCSelector selector,
                                      DeclContext *dc) {
-      // Only class methods can be mapped to constructors.
-      if (!decl->isClassMethod())
-        return None;
-
-      // Said class methods must be in an actual class.
-      auto objcClass = decl->getClassInterface();
-      if (!objcClass)
-        return None;
-
-      DeclName initName;
-      bool hasCustomName;
-
-      // Check whether we're allowed to try.
-      switch (Impl.getFactoryAsInit(objcClass, decl)) {
-      case FactoryAsInitKind::AsInitializer:
-        if (decl->hasAttr<clang::SwiftNameAttr>()) {
-          auto importedName = Impl.importFullName(decl);
-          initName = importedName.Imported;
-          hasCustomName = importedName.HasCustomName;
-          break;
-        }
-        // FIXME: We probably should stop using this codepath. It won't ever
-        // succeed.
-        SWIFT_FALLTHROUGH;
-
-      case FactoryAsInitKind::Infer: {
-        // Check whether the name fits the pattern.
-        bool isSwiftPrivate = decl->hasAttr<clang::SwiftPrivateAttr>();
-        initName =
-            Impl.mapFactorySelectorToInitializerName(selector,
-                                                     objcClass->getName(),
-                                                     isSwiftPrivate);
-        hasCustomName = false;
-        break;
-      }
-      case FactoryAsInitKind::AsClassMethod:
-        return None;
-      }
+      // Import the full name of the method.
+      auto importedName = Impl.importFullName(decl);
 
-      if (!initName)
-        return None;
+      // Check that we imported an initializer name.
+      DeclName initName = importedName;
+      if (initName.getBaseName() != Impl.SwiftContext.Id_init) return None;
 
-      // Check the result type to determine what kind of initializer we can
-      // create (if any).
-      CtorInitializerKind initKind;
-      if (decl->hasRelatedResultType()) {
-        // instancetype factory methods become convenience factory initializers.
-        initKind = CtorInitializerKind::ConvenienceFactory;
-      } else if (auto objcPtr = decl->getReturnType()
-                                  ->getAs<clang::ObjCObjectPointerType>()) {
-        if (objcPtr->getInterfaceDecl() == objcClass) {
-          initKind = CtorInitializerKind::Factory;
-        } else {
-          // FIXME: Could allow a subclass here, but the rest of the compiler
-          // isn't prepared for that yet.
-          // Not a factory method.
-          ++NumFactoryMethodsWrongResult;
-          return None;
-        }
-      } else {
-        // Not a factory method.
-        ++NumFactoryMethodsWrongResult;
+      // ... that came from a factory method.
+      if (importedName.InitKind != CtorInitializerKind::Factory &&
+          importedName.InitKind != CtorInitializerKind::ConvenienceFactory)
         return None;
-      }
 
       bool redundant = false;
-      auto result = importConstructor(decl, dc, false, initKind,
+      auto result = importConstructor(decl, dc, false, importedName.InitKind,
                                       /*required=*/false, selector, initName,
-                                      hasCustomName,
+                                      importedName.HasCustomName,
                                       {decl->param_begin(), decl->param_size()},
                                       decl->isVariadic(), redundant);
       if (result)
@@ -2865,7 +2810,8 @@ namespace {
         // TODO: Could add a replacement string?
         llvm::SmallString<64> message;
         llvm::raw_svector_ostream os(message);
-        os << "use object construction '" << objcClass->getName() << "(";
+        os << "use object construction '"
+           << decl->getClassInterface()->getName() << "(";
         for (auto arg : initName.getArgumentNames()) {
           os << arg << ":";
         }
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -809,22 +809,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   DeclName mapSelectorToDeclName(ObjCSelector selector, bool isInitializer,
                                  bool isSwiftPrivate);
 
-  /// Try to map the given selector, which may be the name of a factory method,
-  /// to the name of an initializer.
-  ///
-  /// \param selector The selector to map.
-  ///
-  /// \param className The name of the class in which the method occurs.
-  ///
-  /// \param isSwiftPrivate Whether this name is for a declaration marked with
-  /// the 'swift_private' attribute.
-  ///
-  /// \returns the initializer name for this factory method, or an empty
-  /// name if this selector does not fit the pattern.
-  DeclName mapFactorySelectorToInitializerName(ObjCSelector selector,
-                                               StringRef className,
-                                               bool isSwiftPrivate);
-
   /// \brief Import the given Swift source location into Clang.
   clang::SourceLocation exportSourceLoc(SourceLoc loc);
 
