[6.3][cxx-interop] Do not drop CV qualifiers during printing template names

Explanation: Dropping qualifiers can result in collisions that can trigger compiler crashes or suprious errors. This PR attempts to make sure we always handle the qualifiers when a qualified type is present in the Clang API. The handling of pointers is somewhat special as foreign reference types has custom printing logic.
Issues: rdar://164917816
Original PRs: #85694
Risk: Low, we preserve more information in the generated template names
so we should have fewer collisions.
Testing: Added a compiler test.
Reviewers: @egorzhdan @j-hui

Author: Xazax-hun

lib/ClangImporter/ClangClassTemplateNamePrinter.cpp

@@ -58,6 +58,21 @@ struct TemplateInstantiationNamePrinter
     return VisitType(type);
   }
 
+  void emitWithCVQualifiers(llvm::raw_svector_ostream &buffer,
+                            clang::QualType type) {
+    if (type.isConstQualified())
+      buffer << "__cxxConst<";
+    if (type.isVolatileQualified())
+      buffer << "__cxxVolatile<";
+
+    buffer << Visit(type.getTypePtr());
+
+    if (type.isVolatileQualified())
+      buffer << ">";
+    if (type.isConstQualified())
+      buffer << ">";
+  }
+
   std::string VisitTagType(const clang::TagType *type) {
     auto tagDecl = type->getAsTagDecl();
     if (auto namedArg = dyn_cast_or_null<clang::NamedDecl>(tagDecl)) {
@@ -103,42 +118,47 @@ struct TemplateInstantiationNamePrinter
     } else {
       buffer << "__cxxRRef<";
     }
-    buffer << Visit(type->getPointeeType().getTypePtr()) << ">";
+
+    emitWithCVQualifiers(buffer, type->getPointeeType());
+
+    buffer << ">";
     return buffer.str().str();
   }
 
   std::string VisitPointerType(const clang::PointerType *type) {
-    std::string pointeeResult = Visit(type->getPointeeType().getTypePtr());
-
-    enum class TagTypeDecorator { None, UnsafePointer, UnsafeMutablePointer };
+    clang::QualType pointee = type->getPointeeType();
+    std::string pointeeResult = Visit(pointee.getTypePtr());
 
     // 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 tagDecl = pointee->getAsTagDecl()) {
       if (auto *rd = dyn_cast<clang::RecordDecl>(tagDecl))
         isReferenceType = recordHasReferenceSemantics(rd, importerImpl);
     }
 
-    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)
+
+    if (pointee.isVolatileQualified())
+      buffer << "__cxxVolatile<";
+
+    if (!isReferenceType) {
+      buffer << (pointee.isConstQualified() ? "UnsafePointer<"
+                                            : "UnsafeMutablePointer<");
+      buffer << pointeeResult;
       buffer << '>';
+    } else {
+      if (pointee.isConstQualified())
+        buffer << "__cxxConst<";
+      buffer << pointeeResult;
+      if (pointee.isConstQualified())
+        buffer << ">";
+    }
+
+    if (pointee.isVolatileQualified())
+      buffer << ">";
 
     return buffer.str().str();
   }
@@ -166,14 +186,23 @@ struct TemplateInstantiationNamePrinter
   }
 
   std::string VisitArrayType(const clang::ArrayType *type) {
-    return (Twine("[") + Visit(type->getElementType().getTypePtr()) + "]")
-        .str();
+    llvm::SmallString<128> storage;
+    llvm::raw_svector_ostream buffer(storage);
+    buffer << "[";
+    emitWithCVQualifiers(buffer, type->getElementType());
+    buffer << "]";
+    return buffer.str().str();
   }
 
   std::string VisitConstantArrayType(const clang::ConstantArrayType *type) {
-    return (Twine("Vector<") + Visit(type->getElementType().getTypePtr()) +
-            ", " + std::to_string(type->getSExtSize()) + ">")
-        .str();
+    llvm::SmallString<128> storage;
+    llvm::raw_svector_ostream buffer(storage);
+    buffer << "Vector<";
+    emitWithCVQualifiers(buffer, type->getElementType());
+    buffer << ", ";
+    buffer << type->getSExtSize();
+    buffer << ">";
+    return buffer.str().str();
   }
 };
 
@@ -197,17 +226,7 @@ struct TemplateArgumentPrinter
                                  llvm::raw_svector_ostream &buffer) {
     auto ty = arg.getAsType();
 
-    if (ty.isConstQualified())
-      buffer << "__cxxConst<";
-    if (ty.isVolatileQualified())
-      buffer << "__cxxVolatile<";
-
-    buffer << typePrinter.Visit(ty.getTypePtr());
-
-    if (ty.isVolatileQualified())
-      buffer << ">";
-    if (ty.isConstQualified())
-      buffer << ">";
+    typePrinter.emitWithCVQualifiers(buffer, ty);
   }
 
   void VisitIntegralTemplateArgument(const clang::TemplateArgument &arg,

test/Interop/Cxx/templates/Inputs/class-template-with-frt.h

@@ -0,0 +1,13 @@
+#pragma once
+
+template<class T>
+struct MagicWrapper {};
+
+class __attribute__((swift_attr("import_reference")))
+__attribute__((swift_attr("retain:immortal")))
+__attribute__((swift_attr("release:immortal"))) Foo {};
+
+using MagicWrapperFrt = MagicWrapper<Foo *>;
+using MagicWrapperConstFrt = MagicWrapper<const Foo *>;
+using MagicWrapperVolatileFrt = MagicWrapper<volatile Foo *>;
+using MagicWrapperVolatileFrtRef = MagicWrapper<volatile Foo &>;

test/Interop/Cxx/templates/Inputs/class-template-with-primitive-argument.h

@@ -15,6 +15,9 @@ struct DoubleWrapper {
   int getValuePlusArg(int arg) const { return m.getValuePlusArg(arg); }
 };
 
+template <class T>
+struct EmptyWrapper {};
+
 typedef MagicWrapper<int> WrappedMagicInt;
 typedef MagicWrapper<const int> WrappedMagicIntConst;
 typedef MagicWrapper<const long> WrappedMagicLongConst;
@@ -26,6 +29,8 @@ typedef MagicWrapper<long[]> WrappedMagicLongArr;
 typedef MagicWrapper<int[123]> WrappedMagicIntFixedSizeArr1;
 typedef MagicWrapper<int[124]> WrappedMagicIntFixedSizeArr2;
 typedef MagicWrapper<std::nullptr_t> WrappedMagicNullPtr;
+typedef MagicWrapper<const int[]> WrappedMagicConstIntArr;
+typedef EmptyWrapper<volatile int &> WrappedVolatileIntRef;
 
 typedef DoubleWrapper<MagicWrapper<int>> DoubleWrappedInt;
 typedef DoubleWrapper<MagicWrapper<const int>> DoubleWrappedIntConst;

test/Interop/Cxx/templates/Inputs/module.modulemap

@@ -187,3 +187,8 @@ module ForwardDeclaredSpecialization {
   header "ForwardDeclaredSpecialization.h"
   requires cplusplus
 }
+
+module ClassTemplateWithFrt {
+  header "class-template-with-frt.h"
+  requires cplusplus
+}

test/Interop/Cxx/templates/class-template-with-frt-module-interface.swift

@@ -0,0 +1,6 @@
+// RUN: %target-swift-ide-test -print-module -module-to-print=ClassTemplateWithFrt -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
+
+// CHECK:  typealias MagicWrapperFrt = MagicWrapper<Foo>
+// CHECK:  typealias MagicWrapperConstFrt = MagicWrapper<__cxxConst<Foo>>
+// CHECK:  typealias MagicWrapperVolatileFrt = MagicWrapper<__cxxVolatile<Foo>>
+// CHECK:  typealias MagicWrapperVolatileFrtRef = MagicWrapper<__cxxLRef<__cxxVolatile<Foo>>>

test/Interop/Cxx/templates/class-template-with-frt.swift

@@ -0,0 +1,10 @@
+// RUN: %target-swift-emit-silgen %s -I %S/Inputs -enable-experimental-cxx-interop -disable-availability-checking
+
+import ClassTemplateWithFrt
+
+func f() {
+    // Used to trigger error while emitting SIL.
+    let _ = MagicWrapperFrt()
+    let _ = MagicWrapperConstFrt()
+    let _ = MagicWrapperVolatileFrtRef()
+}

test/Interop/Cxx/templates/class-template-with-primitive-argument-module-interface.swift

@@ -18,6 +18,8 @@
 // CHECK: typealias WrappedMagicIntFixedSizeArr1 = MagicWrapper<Vector<CInt, 123>>
 // CHECK: typealias WrappedMagicIntFixedSizeArr2 = MagicWrapper<Vector<CInt, 124>>
 // CHECK: typealias WrappedMagicNullPtr = MagicWrapper<__cxxNullPtrT>
+// CHECK: typealias WrappedMagicConstIntArr = MagicWrapper<__cxxConst<[CInt]>>
+// CHECK: typealias WrappedVolatileIntRef = EmptyWrapper<__cxxLRef<__cxxVolatile<CInt>>>
 
 // CHECK: typealias DoubleWrappedInt = DoubleWrapper<MagicWrapper<CInt>>
 // CHECK: typealias DoubleWrappedIntConst = DoubleWrapper<MagicWrapper<__cxxConst<CInt>>>