Addressing comments.

Author: hlopko

include/swift/AST/Decl.h

@@ -3401,14 +3401,41 @@ class EnumDecl final : public NominalTypeDecl {
 /// to get the declared type ("Complex" in the above example).
 class StructDecl final : public NominalTypeDecl {
   SourceLoc StructLoc;
-  // We import C++ class template instantiations as non-generic structs
-  // with a name prefixed with `__CxxTemplateInst`. However for proper
-  // serialization we need to have an access to the bound generic type
-  // that would have produced this instantiation. This field contains
-  // that type.
+
+  // We import C++ class templates as generic structs. Then when in Swift code
+  // we want to substitude generic parameters with actual arguments, we
+  // convert the arguments to C++ equivalents and ask Clang to instantiate the
+  // C++ template. Then we import the C++ class template instantiation
+  // as a non-generic structs with a name prefixed with `__CxxTemplateInst`.
+  //
+  // To reiterate:
+  // 1) We need to have a C++ class template declaration in the Clang AST. This
+  //    declaration is simply imported from a Clang module.
+  // 2) We need a Swift generic struct in the Swift AST. This will provide
+  //    template arguments to Clang.
+  // 3) We produce a C++ class template instantiation in the Clang AST
+  //    using 1) and 2). This declaration does not exist in the Clang module
+  //    AST initially in the general case, it's added there on instantiation.
+  // 4) We import the instantiation as a Swift struct, with the name prefixed
+  //    with `__CxxTemplateInst`.
+  //
+  // This causes a problem for serialization/deserialization of the Swift
+  // module. Imagine the Swift struct from 4) is used in the function return
+  // type. We cannot just serialize the non generic Swift struct, because on
+  // deserialization we would need to find its backing Clang declaration
+  // (the C++ class template instantiation), and it won't be found in the
+  // general case. Only the C++ class template from step 1) is in the Clang
+  // AST.
+  //
+  // What we need is to serialize enough information to be
+  // able to instantiate C++ class template on deserialization. It turns out
+  // that all that information is conveniently covered by the BoundGenericType,
+  // which we store in this field. The field is set during the typechecking at
+  // the time when we instantiate the C++ class template.
   //
-  // The field is set during the typechecking at the time when we
-  // instantiate the C++ class template.
+  // Alternative, and likely better solution long term, is to serialize the
+  // C++ class template instantiation into a synthetic Clang module, and load
+  // this Clang module on deserialization.
   Type TemplateInstantiationType = Type();
 
 public:

lib/Sema/TypeCheckType.cpp

@@ -841,15 +841,15 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
   if (auto clangDecl = decl->getClangDecl()) {
     if (auto classTemplateDecl =
             dyn_cast<clang::ClassTemplateDecl>(clangDecl)) {
-      SmallVector<Type, 2> typesOfgenericArgs;
+      SmallVector<Type, 2> typesOfGenericArgs;
       for (auto typeRepr : generic->getGenericArgs()) {
-        typesOfgenericArgs.push_back(resolution.resolveType(typeRepr));
+        typesOfGenericArgs.push_back(resolution.resolveType(typeRepr));
       }
 
       SmallVector<clang::TemplateArgument, 2> templateArguments;
       std::unique_ptr<TemplateInstantiationError> error =
           ctx.getClangTemplateArguments(
-              classTemplateDecl->getTemplateParameters(), typesOfgenericArgs,
+              classTemplateDecl->getTemplateParameters(), typesOfGenericArgs,
               templateArguments);
 
       if (error) {

lib/Serialization/Deserialization.cpp

@@ -5356,23 +5356,25 @@ class TypeDeserializer {
 
     if (auto clangDecl = nominal->getClangDecl()) {
       if (auto ctd = dyn_cast<clang::ClassTemplateDecl>(clangDecl)) {
-        auto clangImporter = static_cast<ClangImporter *>(nominal->getASTContext().getClangModuleLoader());
+        auto clangImporter = static_cast<ClangImporter *>(
+            nominal->getASTContext().getClangModuleLoader());
 
         SmallVector<Type, 2> typesOfGenericArgs;
         for (auto arg : genericArgs) {
-          typesOfgenericArgs.push_back(arg);
+          typesOfGenericArgs.push_back(arg);
         }
 
         SmallVector<clang::TemplateArgument, 2> templateArguments;
         std::unique_ptr<TemplateInstantiationError> error =
-            ctx.getClangTemplateArguments(
-                ctd->getTemplateParameters(), typesOfgenericArgs,
-                templateArguments);
+            ctx.getClangTemplateArguments(ctd->getTemplateParameters(),
+                                          typesOfGenericArgs,
+                                          templateArguments);
 
         auto instantiation = clangImporter->instantiateCXXClassTemplate(
-          const_cast<clang::ClassTemplateDecl *>(ctd), templateArguments);
+            const_cast<clang::ClassTemplateDecl *>(ctd), templateArguments);
 
-        instantiation->setTemplateInstantiationType(BoundGenericType::get(nominal, parentTy, genericArgs));
+        instantiation->setTemplateInstantiationType(
+            BoundGenericType::get(nominal, parentTy, genericArgs));
         return instantiation->getDeclaredInterfaceType();
       }
     }

test/Interop/Cxx/templates/Inputs/class-template-in-namespace-for-swift-module.h

@@ -2,11 +2,11 @@
 #define TEST_INTEROP_CXX_TEMPLATES_INPUTS_CLASS_TEMPLATE_IN_NAMESPACE_FOR_SWIFT_MODULE_H
 
 namespace Space {
-    template <class T> struct Ship { T t; };
+template <class T> struct Ship { T t; };
 } // namespace Space
 
 namespace Engine {
-    struct Turbojet {};
+struct Turbojet {};
 } // namespace Engine
 
 #endif // TEST_INTEROP_CXX_TEMPLATES_INPUTS_CLASS_TEMPLATE_IN_NAMESPACE_FOR_SWIFT_MODULE_H

test/Interop/Cxx/templates/Inputs/class-template-nested-type-for-swift-module.h

@@ -2,10 +2,9 @@
 #define TEST_INTEROP_CXX_TEMPLATES_INPUTS_CLASS_TEMPLATE_NESTED_TYPE_FOR_SWIFT_MODULE_H
 
 template <class T> struct Ship {
-    struct Engine {
-        T power;
-    };
+  struct Engine {
+    T power;
+  };
 };
 
-
 #endif // TEST_INTEROP_CXX_TEMPLATES_INPUTS_CLASS_TEMPLATE_NESTED_TYPE_FOR_SWIFT_MODULE_H

test/Interop/Cxx/templates/Inputs/class-template-non-type-parameter.h

@@ -5,10 +5,7 @@
 using size_t = __SIZE_TYPE__;
 #endif
 
-template<class T, size_t Size>
-struct MagicArray {
-    T t[Size];
-};
+template <class T, size_t Size> struct MagicArray { T t[Size]; };
 
 typedef MagicArray<int, 2> MagicIntPair;
 

test/Interop/Cxx/templates/class-template-in-namespace-module-interface.swift

@@ -5,4 +5,4 @@
 // CHECK:     init()
 // CHECK:   }
 // CHECK:   typealias Orbiter = Space.__CxxTemplateInstN5Space4ShipIJFvbEEEE
-// CHECK: }
+// CHECK: }