[cxx-interop] Use cached record when possible.

lib/ClangImporter/ImportDecl.cpp

@@ -3300,12 +3300,19 @@ namespace {
 
       // Create the struct declaration and record it.
       auto name = importedName.getDeclName().getBaseIdentifier();
-      auto result = Impl.createDeclWithClangNode<StructDecl>(decl,
-                                 AccessLevel::Public,
-                                 Impl.importSourceLoc(decl->getBeginLoc()),
-                                 name,
-                                 Impl.importSourceLoc(decl->getLocation()),
-                                 None, nullptr, dc);
+      StructDecl *result = nullptr;
+      // Try to find an already-imported struct. This case happens any time
+      // there are nested structs. The "Parent" struct will import the "Child"
+      // struct at which point it attempts to import its decl context which is
+      // the "Parent" struct. Without trying to look up already-imported structs
+      // this will cause an infinite loop.
+      auto alreadyImportedResult =
+          Impl.ImportedDecls.find({decl->getCanonicalDecl(), getVersion()});
+      if (alreadyImportedResult != Impl.ImportedDecls.end())
+        return alreadyImportedResult->second;
+      result = Impl.createDeclWithClangNode<StructDecl>(
+          decl, AccessLevel::Public, Impl.importSourceLoc(decl->getBeginLoc()),
+          name, Impl.importSourceLoc(decl->getLocation()), None, nullptr, dc);
       Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;
 
       // FIXME: Figure out what to do with superclasses in C++. One possible
@@ -3316,6 +3323,7 @@ namespace {
       SmallVector<VarDecl *, 4> members;
       SmallVector<FuncDecl *, 4> methods;
       SmallVector<ConstructorDecl *, 4> ctors;
+      SmallVector<TypeDecl *, 4> nestedTypes;
 
       // FIXME: Import anonymous union fields and support field access when
       // it is nested in a struct.
@@ -3374,27 +3382,13 @@ namespace {
           continue;
         }
 
-        if (isa<TypeDecl>(member)) {
+        if (auto nestedType = dyn_cast<TypeDecl>(member)) {
           // Only import definitions. Otherwise, we might add the same member
           // twice.
           if (auto tagDecl = dyn_cast<clang::TagDecl>(nd))
             if (tagDecl->getDefinition() != tagDecl)
               continue;
-          // A struct nested inside another struct will either be logically
-          // a sibling of the outer struct, or contained inside of it, depending
-          // on if it has a declaration name or not.
-          //
-          // struct foo { struct bar { ... } baz; } // sibling
-          // struct foo { struct { ... } baz; } // child
-          //
-          // In the latter case, we add the imported type as a nested type
-          // of the parent.
-          //
-          // TODO: C++ types have different rules.
-          if (auto nominalDecl = dyn_cast<NominalTypeDecl>(member->getDeclContext())) {
-            assert(nominalDecl == result && "interesting nesting of C types?");
-            nominalDecl->addMember(member);
-          }
+          nestedTypes.push_back(nestedType);
           continue;
         }
 
@@ -3417,7 +3411,30 @@ namespace {
         }
 
         members.push_back(VD);
+      }
 
+      for (auto nestedType : nestedTypes) {
+        // A struct nested inside another struct will either be logically
+        // a sibling of the outer struct, or contained inside of it, depending
+        // on if it has a declaration name or not.
+        //
+        // struct foo { struct bar { ... } baz; } // sibling
+        // struct foo { struct { ... } baz; } // child
+        //
+        // In the latter case, we add the imported type as a nested type
+        // of the parent.
+        //
+        // TODO: C++ types have different rules.
+        if (auto nominalDecl =
+                dyn_cast<NominalTypeDecl>(nestedType->getDeclContext())) {
+          assert(nominalDecl == result && "interesting nesting of C types?");
+          nominalDecl->addMember(nestedType);
+        }
+      }
+
+      bool hasReferenceableFields = !members.empty();
+      for (auto member : members) {
+        auto nd = cast<clang::NamedDecl>(member->getClangDecl());
         // Bitfields are imported as computed properties with Clang-generated
         // accessors.
         bool isBitField = false;
@@ -3428,37 +3445,33 @@ namespace {
             hasUnreferenceableStorage = true;
             isBitField = true;
 
-            makeBitFieldAccessors(Impl,
-                                  const_cast<clang::RecordDecl *>(decl),
-                                  result,
-                                  const_cast<clang::FieldDecl *>(field),
-                                  VD);
+            makeBitFieldAccessors(Impl, const_cast<clang::RecordDecl *>(decl),
+                                  result, const_cast<clang::FieldDecl *>(field),
+                                  member);
           }
         }
 
         if (auto ind = dyn_cast<clang::IndirectFieldDecl>(nd)) {
           // Indirect fields are created as computed property accessible the
           // fields on the anonymous field from which they are injected.
-          makeIndirectFieldAccessors(Impl, ind, members, result, VD);
+          makeIndirectFieldAccessors(Impl, ind, members, result, member);
         } else if (decl->isUnion() && !isBitField) {
           // Union fields should only be available indirectly via a computed
           // property. Since the union is made of all of the fields at once,
           // this is a trivial accessor that casts self to the correct
           // field type.
-          makeUnionFieldAccessors(Impl, result, VD);
+          makeUnionFieldAccessors(Impl, result, member);
 
           // Create labeled initializers for unions that take one of the
           // fields, which only initializes the data for that field.
-          auto valueCtor =
-              createValueConstructor(Impl, result, VD,
-                                     /*want param names*/true,
-                                     /*wantBody=*/true);
+          auto valueCtor = createValueConstructor(Impl, result, member,
+                                                  /*want param names*/ true,
+                                                  /*wantBody=*/true);
           ctors.push_back(valueCtor);
         }
+        result->addMember(member);
       }
 
-      bool hasReferenceableFields = !members.empty();
-
       const clang::CXXRecordDecl *cxxRecordDecl =
           dyn_cast<clang::CXXRecordDecl>(decl);
       if (hasZeroInitializableStorage && !cxxRecordDecl) {
@@ -3486,10 +3499,6 @@ namespace {
         ctors.push_back(valueCtor);
       }
 
-      for (auto member : members) {
-        result->addMember(member);
-      }
-
       for (auto ctor : ctors) {
         result->addMember(ctor);
       }
@@ -9033,7 +9042,9 @@ ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t extra) {
         if (!member)
           continue;
 
-        enumDecl->addMember(member);
+        // TODO: remove this change when #34706 lands.
+        if (!member->NextDecl)
+          enumDecl->addMember(member);
       }
     }
     return;

test/Interop/Cxx/class/Inputs/linked-records.h

@@ -0,0 +1,39 @@
+#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_LINKED_RECORDS_H
+#define TEST_INTEROP_CXX_CLASS_INPUTS_LINKED_RECORDS_H
+
+namespace Space {
+
+class C;
+
+struct A {
+  struct B {
+    B(int) {}
+    B(char) {}
+    B(const C *) {}
+  };
+};
+
+struct C {
+  struct D {
+    A::B B;
+  };
+};
+
+struct E {
+  static void test(const C *);
+};
+
+} // namespace Space
+
+struct M {};
+
+struct F {
+  union {
+    struct {
+    } c;
+    M m;
+  };
+  M m2;
+};
+
+#endif // TEST_INTEROP_CXX_CLASS_INPUTS_LINKED_RECORDS_H

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

@@ -57,3 +57,7 @@ module NestedRecords {
   header "nested-records.h"
   requires cplusplus
 }
+
+module LinkedRecords {
+  header "linked-records.h"
+}

test/Interop/Cxx/class/linked-records-module-interface.swift

@@ -0,0 +1,43 @@
+// RUN: %target-swift-ide-test -print-module -module-to-print=LinkedRecords -I %S/Inputs/ -source-filename=x -enable-cxx-interop | %FileCheck %s
+
+// CHECK: enum Space {
+// CHECK:   struct C {
+// CHECK:     struct D {
+// CHECK:       var B: Space.A.B
+// CHECK:       init(B: Space.A.B)
+// CHECK:     }
+// CHECK:   }
+// CHECK:   struct A {
+// CHECK:     struct B {
+// CHECK:       init(_: Int32)
+// CHECK:       init(_: CChar)
+// CHECK:     }
+// CHECK:     init()
+// CHECK:   }
+// CHECK:   struct E {
+// CHECK:     init()
+// CHECK:     static func test(_: UnsafePointer<Space.C>!)
+// CHECK:   }
+// CHECK: }
+
+// CHECK: struct M {
+// CHECK:   init()
+// CHECK: }
+// CHECK: struct F {
+// CHECK:   struct __Unnamed_union___Anonymous_field0 {
+// CHECK:     struct __Unnamed_struct_c {
+// CHECK:       init()
+// CHECK:     }
+// CHECK:     var c: F.__Unnamed_union___Anonymous_field0.__Unnamed_struct_c
+// CHECK:     var m: M
+// CHECK:     init()
+// CHECK:     init(c: F.__Unnamed_union___Anonymous_field0.__Unnamed_struct_c)
+// CHECK:     init(m: M)
+// CHECK:   }
+// CHECK:   var __Anonymous_field0: F.__Unnamed_union___Anonymous_field0
+// CHECK:   var c: F.__Unnamed_union___Anonymous_field0.__Unnamed_struct_c
+// CHECK:   var m: M
+// CHECK:   var m2: M
+// CHECK:   init()
+// CHECK:   init(_ __Anonymous_field0: F.__Unnamed_union___Anonymous_field0, m2: M)
+// CHECK: }