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

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.

rdar://164917816

Author: Gabor Horvath

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>>>