[cxx-interop] Use unique mangling for distinct C++ class template specializations

egorzhdan · egorzhdan · commit a45d03a6694c · 2023-10-26T13:29:41.000+01:00
This makes sure we are printing more than one level of C++ template specializations when emitting a Swift struct name.

For instance, `std::__wrap_iter&lt;char*&gt;` and `std::__wrap_iter&lt;const char*&gt;` are currently imported with the same name in Swift. This means the mangled string will be the same for these specializations, despite them being distinct types. This causes mangling errors.

rdar://117485399
diff --git a/lib/ClangImporter/CMakeLists.txt b/lib/ClangImporter/CMakeLists.txt
@@ -8,6 +8,7 @@ add_gyb_target(generated_sorted_cf_database
 add_swift_host_library(swiftClangImporter STATIC
   CFTypeInfo.cpp
   ClangAdapter.cpp
+  ClangClassTemplateNamePrinter.cpp
   ClangDerivedConformances.cpp
   ClangDiagnosticConsumer.cpp
   ClangImporter.cpp
diff --git a/lib/ClangImporter/ClangClassTemplateNamePrinter.cpp b/lib/ClangImporter/ClangClassTemplateNamePrinter.cpp
@@ -0,0 +1,147 @@
+//===--- ClangClassTemplateNamePrinter.cpp --------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangClassTemplateNamePrinter.h"
+#include "ImporterImpl.h"
+#include "clang/AST/TypeVisitor.h"
+
+using namespace swift;
+using namespace swift::importer;
+
+struct TemplateInstantiationNamePrinter
+    : clang::TypeVisitor<TemplateInstantiationNamePrinter, std::string> {
+  ASTContext &swiftCtx;
+  NameImporter *nameImporter;
+  ImportNameVersion version;
+
+  TemplateInstantiationNamePrinter(ASTContext &swiftCtx,
+                                   NameImporter *nameImporter,
+                                   ImportNameVersion version)
+      : swiftCtx(swiftCtx), nameImporter(nameImporter), version(version) {}
+
+  std::string VisitBuiltinType(const clang::BuiltinType *type) {
+    Type swiftType = nullptr;
+    switch (type->getKind()) {
+    case clang::BuiltinType::Void:
+      swiftType =
+          swiftCtx.getNamedSwiftType(swiftCtx.getStdlibModule(), "Void");
+      break;
+#define MAP_BUILTIN_TYPE(CLANG_BUILTIN_KIND, SWIFT_TYPE_NAME)                  \
+      case clang::BuiltinType::CLANG_BUILTIN_KIND:                             \
+        swiftType = swiftCtx.getNamedSwiftType(swiftCtx.getStdlibModule(),     \
+                                               #SWIFT_TYPE_NAME);              \
+        break;
+#define MAP_BUILTIN_CCHAR_TYPE(CLANG_BUILTIN_KIND, SWIFT_TYPE_NAME)            \
+      case clang::BuiltinType::CLANG_BUILTIN_KIND:                             \
+        swiftType = swiftCtx.getNamedSwiftType(swiftCtx.getStdlibModule(),     \
+                                               #SWIFT_TYPE_NAME);              \
+        break;
+#include "swift/ClangImporter/BuiltinMappedTypes.def"
+    default:
+      break;
+    }
+
+    if (swiftType) {
+      if (swiftType->is<NominalType>()) {
+        return swiftType->getStringAsComponent();
+      }
+    }
+    return "_";
+  }
+
+  std::string VisitRecordType(const clang::RecordType *type) {
+    auto tagDecl = type->getAsTagDecl();
+    if (auto namedArg = dyn_cast_or_null<clang::NamedDecl>(tagDecl)) {
+      llvm::SmallString<128> storage;
+      llvm::raw_svector_ostream buffer(storage);
+      nameImporter->importName(namedArg, version, clang::DeclarationName())
+          .getDeclName()
+          .print(buffer);
+      return buffer.str().str();
+    }
+    return "_";
+  }
+
+  std::string VisitPointerType(const clang::PointerType *type) {
+    std::string pointeeResult = Visit(type->getPointeeType().getTypePtr());
+
+    enum class TagTypeDecorator { None, UnsafePointer, UnsafeMutablePointer };
+
+    // If this is a pointer to foreign reference type, we should not wrap
+    // it in Unsafe(Mutable)?Pointer, since it will be imported as a class
+    // in Swift.
+    bool isReferenceType = false;
+    if (auto tagDecl = type->getPointeeType()->getAsTagDecl()) {
+      if (auto *rd = dyn_cast<clang::RecordDecl>(tagDecl))
+        isReferenceType =
+            ClangImporter::Implementation::recordHasReferenceSemantics(
+                rd, swiftCtx);
+    }
+
+    TagTypeDecorator decorator;
+    if (!isReferenceType)
+      decorator = type->getPointeeType().isConstQualified()
+                      ? TagTypeDecorator::UnsafePointer
+                      : TagTypeDecorator::UnsafeMutablePointer;
+    else
+      decorator = TagTypeDecorator::None;
+
+    llvm::SmallString<128> storage;
+    llvm::raw_svector_ostream buffer(storage);
+    if (decorator != TagTypeDecorator::None)
+      buffer << (decorator == TagTypeDecorator::UnsafePointer
+                     ? "UnsafePointer"
+                     : "UnsafeMutablePointer")
+             << '<';
+    buffer << pointeeResult;
+    if (decorator != TagTypeDecorator::None)
+      buffer << '>';
+
+    return buffer.str().str();
+  }
+};
+
+std::string swift::importer::printClassTemplateSpecializationName(
+    const clang::ClassTemplateSpecializationDecl *decl, ASTContext &swiftCtx,
+    NameImporter *nameImporter, ImportNameVersion version) {
+  TemplateInstantiationNamePrinter templateNamePrinter(swiftCtx, nameImporter,
+                                                       version);
+
+  // TODO: the following logic should probably be a ConstTemplateArgumentVisitor
+  llvm::SmallString<128> storage;
+  llvm::raw_svector_ostream buffer(storage);
+  decl->printName(buffer);
+  buffer << "<";
+  llvm::interleaveComma(
+      decl->getTemplateArgs().asArray(), buffer,
+      [&buffer, &templateNamePrinter](const clang::TemplateArgument &arg) {
+        // Use import name here so builtin types such as "int" map to their
+        // Swift equivalent ("CInt").
+        if (arg.getKind() == clang::TemplateArgument::Type) {
+          auto ty = arg.getAsType().getTypePtr();
+          buffer << templateNamePrinter.Visit(ty);
+          return;
+        } else if (arg.getKind() == clang::TemplateArgument::Integral) {
+          buffer << "_";
+          if (arg.getIntegralType()->isBuiltinType()) {
+            buffer << templateNamePrinter.Visit(
+                          arg.getIntegralType().getTypePtr())
+                   << "_";
+          }
+          arg.getAsIntegral().print(buffer, true);
+          return;
+        }
+        buffer << "_";
+      });
+  buffer << ">";
+  return buffer.str().str();
+}
diff --git a/lib/ClangImporter/ClangClassTemplateNamePrinter.h b/lib/ClangImporter/ClangClassTemplateNamePrinter.h
@@ -0,0 +1,40 @@
+//===--- ClangClassTemplateNamePrinter.h ------------------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_CLANG_TEMPLATE_NAME_PRINTER_H
+#define SWIFT_CLANG_TEMPLATE_NAME_PRINTER_H
+
+#include "ImportName.h"
+#include "swift/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
+
+namespace swift {
+namespace importer {
+
+/// Returns a Swift representation of a C++ class template specialization name,
+/// e.g. "vector<CWideChar, allocator<CWideChar>>".
+///
+/// This expands the entire tree of template instantiation names recursively.
+/// While printing deep instantiation levels might not increase readability, it
+/// is important to do because the C++ templated class names get mangled,
+/// therefore they must be unique for different instantiations.
+///
+/// This function does not instantiate any templates and does not modify the AST
+/// in any way.
+std::string printClassTemplateSpecializationName(
+    const clang::ClassTemplateSpecializationDecl *decl, ASTContext &swiftCtx,
+    NameImporter *nameImporter, ImportNameVersion version);
+
+} // namespace importer
+} // namespace swift
+
+#endif // SWIFT_CLANG_TEMPLATE_NAME_PRINTER_H
diff --git a/lib/ClangImporter/ImportName.cpp b/lib/ClangImporter/ImportName.cpp
@@ -16,6 +16,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "CFTypeInfo.h"
+#include "ClangClassTemplateNamePrinter.h"
 #include "ClangDiagnosticConsumer.h"
 #include "ImporterImpl.h"
 #include "swift/AST/ASTContext.h"
@@ -2214,111 +2215,9 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
       return importNameImpl(classTemplateSpecDecl->getSpecializedTemplate(),
                             version, givenName);
     if (!isa<clang::ClassTemplatePartialSpecializationDecl>(D)) {
-      auto getSwiftBuiltinTypeName =
-          [&](const clang::BuiltinType *builtin) -> std::optional<std::string> {
-        Type swiftType = nullptr;
-        switch (builtin->getKind()) {
-        case clang::BuiltinType::Void:
-          swiftType = swiftCtx.getNamedSwiftType(swiftCtx.getStdlibModule(),
-                                                 "Void");
-          break;
-#define MAP_BUILTIN_TYPE(CLANG_BUILTIN_KIND, SWIFT_TYPE_NAME)                  \
-        case clang::BuiltinType::CLANG_BUILTIN_KIND:                           \
-          swiftType = swiftCtx.getNamedSwiftType(swiftCtx.getStdlibModule(),   \
-                                                 #SWIFT_TYPE_NAME);            \
-          break;
-#define MAP_BUILTIN_CCHAR_TYPE(CLANG_BUILTIN_KIND, SWIFT_TYPE_NAME)            \
-        case clang::BuiltinType::CLANG_BUILTIN_KIND:                           \
-          swiftType = swiftCtx.getNamedSwiftType(swiftCtx.getStdlibModule(),   \
-                                                 #SWIFT_TYPE_NAME);            \
-          break;
-#include "swift/ClangImporter/BuiltinMappedTypes.def"
-        default:
-          break;
-        }
-
-        if (swiftType) {
-          if (swiftType->is<NominalType>()) {
-            return swiftType->getStringAsComponent();
-          }
-        }
-        return std::nullopt;
-      };
-
-      // When constructing the name of a C++ template, don't expand all the
-      // template, only expand one layer. Here we want to prioritize
-      // readability over total completeness.
-      llvm::SmallString<128> storage;
-      llvm::raw_svector_ostream buffer(storage);
-      D->printName(buffer);
-      buffer << "<";
-      llvm::interleaveComma(classTemplateSpecDecl->getTemplateArgs().asArray(),
-                            buffer,
-                            [&buffer, this, version, &getSwiftBuiltinTypeName](const clang::TemplateArgument& arg) {
-        // Use import name here so builtin types such as "int" map to their
-        // Swift equivalent ("Int32").
-        if (arg.getKind() == clang::TemplateArgument::Type) {
-          auto ty = arg.getAsType().getTypePtr();
-          if (auto builtin = dyn_cast<clang::BuiltinType>(ty)) {
-            if (auto swiftTypeName = getSwiftBuiltinTypeName(builtin)) {
-              buffer << *swiftTypeName;
-              return;
-            }
-          } else {
-            // FIXME: Generalize this to cover pointer to
-            // builtin type too.
-            // Check if this a struct/class
-            // or a pointer/reference to a struct/class.
-            auto *tagDecl = ty->getAsTagDecl();
-            enum class TagTypeDecorator {
-              None,
-              UnsafePointer,
-              UnsafeMutablePointer
-            };
-            TagTypeDecorator decorator = TagTypeDecorator::None;
-            if (!tagDecl && ty->isPointerType()) {
-              tagDecl = ty->getPointeeType()->getAsTagDecl();
-              if (tagDecl) {
-                bool isReferenceType = false;
-                if (auto *rd = dyn_cast<clang::RecordDecl>(tagDecl))
-                  isReferenceType = ClangImporter::Implementation::
-                      recordHasReferenceSemantics(rd, swiftCtx);
-                if (!isReferenceType)
-                  decorator = ty->getPointeeType().isConstQualified()
-                                  ? TagTypeDecorator::UnsafePointer
-                                  : TagTypeDecorator::UnsafeMutablePointer;
-              }
-            }
-            if (auto namedArg = dyn_cast_or_null<clang::NamedDecl>(tagDecl)) {
-              if (decorator != TagTypeDecorator::None)
-                buffer << (decorator == TagTypeDecorator::UnsafePointer
-                               ? "UnsafePointer"
-                               : "UnsafeMutablePointer")
-                       << '<';
-              importNameImpl(namedArg, version, clang::DeclarationName())
-                  .getDeclName()
-                  .print(buffer);
-              if (decorator != TagTypeDecorator::None)
-                buffer << '>';
-              return;
-            }
-          }
-        } else if (arg.getKind() == clang::TemplateArgument::Integral) {
-          buffer << "_";
-          if (arg.getIntegralType()->isBuiltinType()) {
-            if (auto swiftTypeName = getSwiftBuiltinTypeName(
-                    arg.getIntegralType()->getAs<clang::BuiltinType>())) {
-              buffer << *swiftTypeName << "_";
-            }
-          }
-          arg.getAsIntegral().print(buffer, true);
-          return;
-        }
-        buffer << "_";
-      });
-      buffer << ">";
-
-      baseName = swiftCtx.getIdentifier(buffer.str()).get();
+      auto name = printClassTemplateSpecializationName(classTemplateSpecDecl,
+                                                       swiftCtx, this, version);
+      baseName = swiftCtx.getIdentifier(name).get();
     }
   }
 
diff --git a/test/Interop/Cxx/templates/Inputs/class-template-with-primitive-argument.h b/test/Interop/Cxx/templates/Inputs/class-template-with-primitive-argument.h
@@ -8,5 +8,8 @@ struct MagicWrapper {
 };
 
 typedef MagicWrapper<int> WrappedMagicInt;
+typedef MagicWrapper<int*> WrappedMagicIntPtr;
+typedef MagicWrapper<const int*> WrappedMagicIntConstPtr;
+typedef MagicWrapper<int**> WrappedMagicIntPtrPtr;
 
 #endif // TEST_INTEROP_CXX_TEMPLATES_INPUTS_CLASS_TEMPLATE_WITH_PRIMITIVE_ARGUMENT_H
diff --git a/test/Interop/Cxx/templates/class-template-with-primitive-argument-module-interface.swift b/test/Interop/Cxx/templates/class-template-with-primitive-argument-module-interface.swift
@@ -0,0 +1,10 @@
+// RUN: %target-swift-ide-test -print-module -module-to-print=ClassTemplateWithPrimitiveArgument -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
+
+// CHECK: @available(*, unavailable
+// CHECK: struct MagicWrapper<T> {
+// CHECK: }
+
+// CHECK: typealias WrappedMagicInt = MagicWrapper<CInt>
+// CHECK: typealias WrappedMagicIntPtr = MagicWrapper<UnsafeMutablePointer<CInt>>
+// CHECK: typealias WrappedMagicIntConstPtr = MagicWrapper<UnsafePointer<CInt>>
+// CHECK: typealias WrappedMagicIntPtrPtr = MagicWrapper<UnsafeMutablePointer<UnsafeMutablePointer<CInt>>>
diff --git a/test/SILGen/opaque_values_cxx.swift b/test/SILGen/opaque_values_cxx.swift
@@ -15,12 +15,12 @@ import Cxx
 // CHECK:         end_borrow [[VECTOR]]
 // CHECK:         return [[BEGIN]]
 // CHECK-LABEL: } // end sil function '$sSo3stdO3__1O0055vectorCUnsignedIntallocatorCUnsignedInt_iqGBpboaivxaEhaV3Cxx0B8SequenceSCAgHP13__beginUnsafe11RawIteratorQzyFTW'
-// CHECK-LABEL: sil {{.*}}[ossa] @$sSo3stdO{{(3__1O)?}}0020___wrap_iter__udAAdDaVSQSCSQ2eeoiySbx_xtFZTW : {{.*}} {
-// CHECK:       bb0([[LHS:%[^,]+]] : $std.__1.__wrap_iter<_>, [[RHS:%[^,]+]] :
-// CHECK:         [[CALLEE:%[^,]+]] = function_ref @$sSo2eeoiySbSo3stdO{{(3__1O)?}}0020___wrap_iter__udAAdDaV_AGtFTO
+// CHECK-LABEL: sil {{.*}}[ossa] @$sSo3stdO{{(3__1O)?}}0047___wrap_iterUnsafePointerCUnsignedInt_heCInnaEgaVSQSCSQ2eeoiySbx_xtFZTW : {{.*}} {
+// CHECK:       bb0([[LHS:%[^,]+]] : $std.__1.__wrap_iter<UnsafePointer<CUnsignedInt>>, [[RHS:%[^,]+]] :
+// CHECK:         [[CALLEE:%[^,]+]] = function_ref @$sSo2eeoiySbSo3stdO{{(3__1O)?}}0047___wrap_iterUnsafePointerCUnsignedInt_heCInnaEgaV_AGtFTO
 // CHECK:         [[EQUAL:%[^,]+]] = apply [[CALLEE]]([[LHS]], [[RHS]])
 // CHECK:         return [[EQUAL]]
-// CHECK-LABEL: } // end sil function '$sSo3stdO{{(3__1O)?}}0020___wrap_iter__udAAdDaVSQSCSQ2eeoiySbx_xtFZTW'
+// CHECK-LABEL: } // end sil function '$sSo3stdO{{(3__1O)?}}0047___wrap_iterUnsafePointerCUnsignedInt_heCInnaEgaVSQSCSQ2eeoiySbx_xtFZTW'
 func test_cxx_vector_uint32t_iterate(_ n: Int, _ vectorOfU32: VectorOfU32) {
   for x in vectorOfU32 {}
 }
