[interop][SwiftToCxx] avoid emitting ambiguous C++ overloads

Just do an arity check for now

Author: hyp

lib/AST/SwiftNameTranslation.cpp

@@ -164,6 +164,9 @@ swift::cxx_translation::getNameForCxx(const ValueDecl *VD,
   if (customNamesOnly)
     return StringRef();
 
+  if (isa<ConstructorDecl>(VD))
+    return "init";
+
   if (auto *mod = dyn_cast<ModuleDecl>(VD)) {
     if (mod->isStdlibModule())
       return "swift";

lib/PrintAsClang/DeclAndTypePrinter.cpp

@@ -1012,6 +1012,39 @@ class DeclAndTypePrinter::Implementation
            sel.getSelectorPieces().front().str() == "init";
   }
 
+  /// Returns true if the given function overload is safe to emit in the current
+  /// C++ lexical scope.
+  bool canPrintOverloadOfFunction(const AbstractFunctionDecl *funcDecl) const {
+    assert(outputLang == OutputLanguageMode::Cxx);
+    auto &overloads =
+        owningPrinter.getCxxDeclEmissionScope().emittedFunctionOverloads;
+    auto cxxName = cxx_translation::getNameForCxx(funcDecl);
+    auto overloadIt = overloads.find(cxxName);
+    if (overloadIt == overloads.end()) {
+      overloads.insert(std::make_pair(
+          cxxName,
+          llvm::SmallVector<const AbstractFunctionDecl *>({funcDecl})));
+      return true;
+    }
+    auto selfArity =
+        funcDecl->getParameters() ? funcDecl->getParameters()->size() : 0;
+    for (const auto *overload : overloadIt->second) {
+      auto arity =
+          overload->getParameters() ? overload->getParameters()->size() : 0;
+      // Avoid printing out an overload with the same and arity, as that might
+      // be an ambiguous overload on the C++ side.
+      // FIXME: we should take types into account, not all overloads with the
+      // same arity are ambiguous in C++.
+      if (selfArity == arity) {
+        owningPrinter.getCxxDeclEmissionScope()
+            .additionalUnrepresentableDeclarations.push_back(funcDecl);
+        return false;
+      }
+    }
+    overloadIt->second.push_back(funcDecl);
+    return true;
+  }
+
   void printAbstractFunctionAsMethod(AbstractFunctionDecl *AFD,
                                      bool isClassMethod,
                                      bool isNSUIntegerSubscript = false,
@@ -1051,6 +1084,12 @@ class DeclAndTypePrinter::Implementation
         if (!dispatchInfo)
           return;
       }
+      // FIXME: handle getters/setters ambiguities here too.
+      if (!isa<AccessorDecl>(AFD)) {
+        if (!canPrintOverloadOfFunction(AFD))
+          return;
+      }
+
       owningPrinter.prologueOS << cFuncPrologueOS.str();
 
       printDocumentationComment(AFD);
@@ -1779,6 +1818,8 @@ class DeclAndTypePrinter::Implementation
             .additionalUnrepresentableDeclarations.push_back(FD);
         return;
       }
+      if (!canPrintOverloadOfFunction(FD))
+        return;
       owningPrinter.prologueOS << cFuncPrologueOS.str();
       printAbstractFunctionAsCxxFunctionThunk(FD, *funcABI);
       recordEmittedDeclInCurrentCxxLexicalScope(FD);

lib/PrintAsClang/DeclAndTypePrinter.h

@@ -37,6 +37,10 @@ struct CxxDeclEmissionScope {
   std::vector<const ValueDecl *> additionalUnrepresentableDeclarations;
   /// Records the C++ declaration names already emitted in this lexical scope.
   llvm::StringSet<> emittedDeclarationNames;
+  /// Records the names of the function overloads already emitted in this
+  /// lexical scope.
+  llvm::StringMap<llvm::SmallVector<const AbstractFunctionDecl *, 2>>
+      emittedFunctionOverloads;
 };
 
 /// Responsible for printing a Swift Decl or Type in Objective-C, to be

lib/PrintAsClang/ModuleContentsWriter.cpp

@@ -27,6 +27,7 @@
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SwiftNameTranslation.h"
 #include "swift/AST/TypeDeclFinder.h"
+#include "swift/Basic/SourceManager.h"
 #include "swift/ClangImporter/ClangImporter.h"
 #include "swift/Strings.h"
 
@@ -676,8 +677,26 @@ class ModuleWriter {
       if (result != 0)
         return result;
       // Two overloaded functions can have the same name when emitting C++.
-      if (isa<AbstractFunctionDecl>(*rhs) && isa<AbstractFunctionDecl>(*lhs))
+      if (isa<AbstractFunctionDecl>(*rhs) && isa<AbstractFunctionDecl>(*lhs)) {
+        // Sort top level functions with the same C++ name by their location to
+        // have stable sorting that depends on users source but not on the
+        // compiler invocation.
+        if ((*rhs)->getLoc().isValid() && (*lhs)->getLoc().isValid()) {
+          std::string rhsLoc, lhsLoc;
+          auto getLocText = [](const AbstractFunctionDecl *afd) {
+            std::string res;
+            llvm::raw_string_ostream os(res);
+            afd->getLoc().print(os, afd->getASTContext().SourceMgr);
+            return std::move(os.str());
+          };
+          if (getLocText(cast<AbstractFunctionDecl>(*lhs)) <
+              getLocText(cast<AbstractFunctionDecl>(*rhs)))
+            return Descending;
+          return Ascending;
+        }
         return result;
+      }
+
       // A function and a global variable can have the same name in C++,
       // even when the variable might not actually be emitted by the emitter.
       // In that case, order the function before the variable.

lib/PrintAsClang/PrintClangFunction.cpp

@@ -1369,16 +1369,6 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
   }
 }
 
-static StringRef getConstructorName(const AbstractFunctionDecl *FD) {
-  auto name = cxx_translation::getNameForCxx(
-      FD, cxx_translation::CustomNamesOnly_t::CustomNamesOnly);
-  if (!name.empty()) {
-    assert(name.startswith("init"));
-    return name;
-  }
-  return "init";
-}
-
 void DeclAndTypeClangFunctionPrinter::printCxxMethod(
     DeclAndTypePrinter &declAndTypePrinter,
     const NominalTypeDecl *typeDeclContext, const AbstractFunctionDecl *FD,
@@ -1399,10 +1389,8 @@ void DeclAndTypeClangFunctionPrinter::printCxxMethod(
                       !isConstructor && !isStatic;
   modifiers.hasSymbolUSR = !isDefinition;
   auto result = printFunctionSignature(
-      FD, signature,
-      isConstructor ? getConstructorName(FD)
-                    : cxx_translation::getNameForCxx(FD),
-      resultTy, FunctionSignatureKind::CxxInlineThunk, modifiers);
+      FD, signature, cxx_translation::getNameForCxx(FD), resultTy,
+      FunctionSignatureKind::CxxInlineThunk, modifiers);
   assert(!result.isUnsupported() && "C signature should be unsupported too");
 
   declAndTypePrinter.printAvailability(os, FD);

test/Interop/SwiftToCxx/functions/swift-function-overloads.swift

@@ -4,15 +4,15 @@
 
 // RUN: %check-interop-cxx-header-in-clang(%t/functions.h)
 
-
-
 public func overloadedFunc(_ x: Int) { }
 public func overloadedFunc(_ y: Float) { }
 
 public func overloadedFuncArgLabel(x _: Int) { }
 public func overloadedFuncArgLabel(y _: Float) { }
 
-// CHECK-DAG: void overloadedFunc(float y) noexcept
-// CHECK-DAG: void overloadedFunc(swift::Int x) noexcept
-// CHECK-DAG: void overloadedFuncArgLabel(float _1) noexcept
-// CHECK-DAG: void overloadedFuncArgLabel(swift::Int _1) noexcept
+// CHECK: void overloadedFunc(swift::Int x) noexcept
+// CHECK: void overloadedFuncArgLabel(swift::Int  _1) noexcept
+
+// CHECK: // Unavailable in C++: Swift global function 'overloadedFunc(_:)'.
+
+// CHECK: // Unavailable in C++: Swift global function 'overloadedFuncArgLabel(y:)'.

test/Interop/SwiftToCxx/initializers/init-in-cxx.swift

@@ -126,6 +126,21 @@ public class DerivedClass: BaseClass {
 public class DerivedClassTwo: BaseClass {
 }
 
+public struct WrapOverloadedInits {
+    let x: Int
+
+    public init(_ x: Int) {
+        self.x = x
+    }
+    public init(_ x: Float) {
+        self.x = Int(x)
+    }
+}
+
+// CHECK: static SWIFT_INLINE_THUNK WrapOverloadedInits init
+// CHECK-SAME: (swift::Int x) SWIFT_SYMBOL("s:4Init19WrapOverloadedInitsVyACSicfc");
+// CHECK-NOT: WrapOverloadedInits init(
+
 // CHECK: BaseClass BaseClass::init(swift::Int x, swift::Int y) {
 // CHECK-NEXT: return _impl::_impl_BaseClass::makeRetained(_impl::$s4Init9BaseClassCyACSi_SitcfC(x, y, swift::TypeMetadataTrait<BaseClass>::getTypeMetadata()));
 

test/Interop/SwiftToCxx/methods/method-in-cxx.swift

@@ -103,6 +103,35 @@ public final class PassStructInClassMethod {
 // CHECK-NEXT: static SWIFT_INLINE_THUNK void staticMethod() SWIFT_SYMBOL("s:7Methods11LargeStructV12staticMethodyyFZ");
 // CHECK-NEXT: private
 
+public struct WrapOverloadedMethods {
+    let x: Int
+
+    public func method(_ x: Int) {
+    }
+    public func method(_ x: Float) {
+    }
+    public func method(argLabel y: Int64) {
+    }
+}
+
+// CHECK: WrapOverloadedMethods final {
+// CHECK: SWIFT_INLINE_THUNK void method
+// CHECK-SAME: (swift::Int x) const SWIFT_SYMBOL("s:7Methods014WrapOverloadedA0V6methodyySiF");
+// CHECK-NEXT: private:
+
+public struct WrapOverloadedMethodsSibling {
+    let x: Int
+
+    // Sibling method with same name should be emitted.
+    public func method(_ x: Int) {
+    }
+}
+
+// CHECK: WrapOverloadedMethodsSibling final {
+// CHECK: SWIFT_INLINE_THUNK void method
+// CHECK-SAME: (swift::Int x) const SWIFT_SYMBOL("s:7Methods014WrapOverloadedA7SiblingV6methodyySiF");
+// CHECK-NEXT: private:
+
 public func createClassWithMethods(_ x: Int) -> ClassWithMethods {
     return ClassWithMethods(x)
 }