[Clang importer] Import incomplete SWIFT_SHARED_REFERENCE types

Normally, Swift cannot import an incomplete type. However, when we are
importing a SWIFT_SHARED_REFERENCE type, we're always dealing with
pointers to the type, and there is no need for the underlying type to
be complete. This permits a common pattern in C libraries where the
actual underlying storage is opaque and all APIs traffic in the
pointer, e.g.,

    typedef struct MyTypeImpl *MyType;
    void MyTypeRetain(MyType ptr);
    void MyTypeRelease(MyType ptr);

to use SWIFT_SHARED_REFERENCE to import such types as foreign
references, rather than as OpaquePointer.

Fixes rdar://155970441.

include/swift/ClangImporter/ClangImporter.h

@@ -706,6 +706,10 @@ getCxxReferencePointeeTypeOrNone(const clang::Type *type);
 /// Returns true if the given type is a C++ `const` reference type.
 bool isCxxConstReferenceType(const clang::Type *type);
 
+/// Determine whether the given Clang record declaration has one of the
+/// attributes that makes it import as a reference types.
+bool hasImportAsRefAttr(const clang::RecordDecl *decl);
+
 /// Determine whether this typedef is a CF type.
 bool isCFTypeDecl(const clang::TypedefNameDecl *Decl);
 
@@ -804,16 +808,18 @@ std::optional<T> matchSwiftAttrConsideringInheritance(
 
   if (const auto *recordDecl = llvm::dyn_cast<clang::CXXRecordDecl>(decl)) {
     std::optional<T> result;
-    recordDecl->forallBases([&](const clang::CXXRecordDecl *base) -> bool {
-      if (auto baseMatch = matchSwiftAttr<T>(base, patterns)) {
-        result = baseMatch;
-        return false;
-      }
+    if (recordDecl->isCompleteDefinition()) {
+      recordDecl->forallBases([&](const clang::CXXRecordDecl *base) -> bool {
+        if (auto baseMatch = matchSwiftAttr<T>(base, patterns)) {
+          result = baseMatch;
+          return false;
+        }
 
-      return true;
-    });
+        return true;
+      });
 
-    return result;
+      return result;
+    }
   }
 
   return std::nullopt;

lib/ClangImporter/ClangImporter.cpp

@@ -5312,7 +5312,8 @@ ClangTypeEscapability::evaluate(Evaluator &evaluator,
     if (desc.annotationOnly)
       return CxxEscapability::Unknown;
     auto cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(recordDecl);
-    if (!cxxRecordDecl || cxxRecordDecl->isAggregate()) {
+    if (recordDecl->getDefinition() &&
+        (!cxxRecordDecl || cxxRecordDecl->isAggregate())) {
       if (cxxRecordDecl) {
         for (auto base : cxxRecordDecl->bases()) {
           auto baseEscapability = evaluateEscapability(
@@ -6346,8 +6347,9 @@ TinyPtrVector<ValueDecl *> ClangRecordMemberLookup::evaluate(
   }
 
   // If this is a C++ record, look through any base classes.
-  if (auto cxxRecord =
-          dyn_cast<clang::CXXRecordDecl>(recordDecl->getClangDecl())) {
+  const clang::CXXRecordDecl *cxxRecord;
+  if ((cxxRecord = dyn_cast<clang::CXXRecordDecl>(recordDecl->getClangDecl())) &&
+      cxxRecord->isCompleteDefinition()) {
     // Capture the arity of already found members in the
     // current record, to avoid adding ambiguous members
     // from base classes.
@@ -7813,7 +7815,7 @@ ClangImporter::createEmbeddedBridgingHeaderCacheKey(
                    "ChainedHeaderIncludeTree -> EmbeddedHeaderIncludeTree");
 }
 
-static bool hasImportAsRefAttr(const clang::RecordDecl *decl) {
+bool importer::hasImportAsRefAttr(const clang::RecordDecl *decl) {
   return decl->hasAttrs() && llvm::any_of(decl->getAttrs(), [](auto *attr) {
            if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr))
              return swiftAttr->getAttribute() == "import_reference" ||

lib/ClangImporter/ImportDecl.cpp

@@ -846,6 +846,10 @@ using MirroredMethodEntry =
   std::tuple<const clang::ObjCMethodDecl*, ProtocolDecl*, bool /*isAsync*/>;
 
 static bool areRecordFieldsComplete(const clang::CXXRecordDecl *decl) {
+  // If the type is incomplete, then the fields are not complete.
+  if (!decl->isCompleteDefinition())
+    return false;
+
   for (const auto *f : decl->fields()) {
     auto *fieldRecord = f->getType()->getAsCXXRecordDecl();
     if (fieldRecord) {
@@ -2099,18 +2103,21 @@ namespace {
       if (decl->isInterface())
         return nullptr;
 
-      if (!decl->getDefinition()) {
+      bool incompleteTypeAsReference = false;
+      if (auto def = decl->getDefinition()) {
+        // Continue with the definition of the type.
+        decl = def;
+      } else if (recordHasReferenceSemantics(decl)) {
+        // Incomplete types are okay if the resulting type has reference
+        // semantics.
+        incompleteTypeAsReference = true;
+      } else {
         Impl.addImportDiagnostic(
             decl,
             Diagnostic(diag::incomplete_record, Impl.SwiftContext.AllocateCopy(
                                                     decl->getNameAsString())),
             decl->getLocation());
-      }
 
-      // FIXME: Figure out how to deal with incomplete types, since that
-      // notion doesn't exist in Swift.
-      decl = decl->getDefinition();
-      if (!decl) {
         forwardDeclaration = true;
         return nullptr;
       }
@@ -2126,7 +2133,7 @@ namespace {
       }
 
       // Don't import nominal types that are over-aligned.
-      if (Impl.isOverAligned(decl)) {
+      if (decl->isCompleteDefinition() && Impl.isOverAligned(decl)) {
         Impl.addImportDiagnostic(
             decl, Diagnostic(
                       diag::record_over_aligned,
@@ -2137,6 +2144,9 @@ namespace {
 
       auto isNonTrivialDueToAddressDiversifiedPtrAuth =
           [](const clang::RecordDecl *decl) {
+            if (!decl->isCompleteDefinition())
+              return true;
+
             for (auto *field : decl->fields()) {
               if (!field->getType().isNonTrivialToPrimitiveCopy()) {
                 continue;
@@ -2192,7 +2202,8 @@ namespace {
                                             *correctSwiftName);
 
       auto dc =
-          Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
+          Impl.importDeclContextOf(decl, importedName.getEffectiveContext(),
+                                   incompleteTypeAsReference);
       if (!dc) {
         Impl.addImportDiagnostic(
             decl, Diagnostic(
@@ -2239,9 +2250,11 @@ namespace {
       // solution would be to turn them into members and add conversion
       // functions.
       if (auto cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(decl)) {
-        for (auto base : cxxRecordDecl->bases()) {
-          if (auto *baseRecordDecl = base.getType()->getAsCXXRecordDecl()) {
-            Impl.importDecl(baseRecordDecl, getVersion());
+        if (cxxRecordDecl->isCompleteDefinition()) {
+          for (auto base : cxxRecordDecl->bases()) {
+            if (auto *baseRecordDecl = base.getType()->getAsCXXRecordDecl()) {
+              Impl.importDecl(baseRecordDecl, getVersion());
+            }
           }
         }
       }
@@ -2449,7 +2462,9 @@ namespace {
 
       const clang::CXXRecordDecl *cxxRecordDecl =
           dyn_cast<clang::CXXRecordDecl>(decl);
-      bool hasBaseClasses = cxxRecordDecl && !cxxRecordDecl->bases().empty();
+      bool hasBaseClasses = cxxRecordDecl &&
+          cxxRecordDecl->isCompleteDefinition() &&
+          !cxxRecordDecl->bases().empty();
       if (hasBaseClasses) {
         hasUnreferenceableStorage = true;
         hasMemberwiseInitializer = false;
@@ -2457,7 +2472,8 @@ namespace {
 
       bool needsEmptyInitializer = true;
       if (cxxRecordDecl) {
-        needsEmptyInitializer = !cxxRecordDecl->isAbstract() &&
+        needsEmptyInitializer = cxxRecordDecl->isCompleteDefinition() &&
+                                !cxxRecordDecl->isAbstract() &&
                                 (!cxxRecordDecl->hasDefaultConstructor() ||
                                  cxxRecordDecl->ctors().empty());
       }
@@ -2501,7 +2517,8 @@ namespace {
       // only when the same is possible in C++. While we could check for that
       // exactly, checking whether the C++ class is an aggregate
       // (C++ [dcl.init.aggr]) has the same effect.
-      bool isAggregate = !cxxRecordDecl || cxxRecordDecl->isAggregate();
+      bool isAggregate = decl->isCompleteDefinition() &&
+          (!cxxRecordDecl || cxxRecordDecl->isAggregate());
       if ((hasReferenceableFields && hasMemberwiseInitializer && isAggregate) ||
           forceMemberwiseInitializer) {
         // The default zero initializer suppresses the implicit value
@@ -2905,7 +2922,13 @@ namespace {
       if (!Impl.SwiftContext.LangOpts.EnableCXXInterop)
         return VisitRecordDecl(decl);
 
-      if (!decl->getDefinition()) {
+      if (auto def = decl->getDefinition()) {
+        // Continue with the definition of the type.
+        decl = def;
+      } else if (recordHasReferenceSemantics(decl)) {
+        // Incomplete types are okay if the resulting type has reference
+        // semantics.
+      } else {
         Impl.addImportDiagnostic(
             decl,
             Diagnostic(diag::incomplete_record, Impl.SwiftContext.AllocateCopy(
@@ -2921,10 +2944,7 @@ namespace {
             Impl.diagnose(HeaderLoc(attr.second),
                           diag::private_fileid_attr_here);
         }
-      }
 
-      decl = decl->getDefinition();
-      if (!decl) {
         forwardDeclaration = true;
         return nullptr;
       }
@@ -3141,12 +3161,14 @@ namespace {
     void
     addExplicitProtocolConformances(NominalTypeDecl *decl,
                                     const clang::CXXRecordDecl *clangDecl) {
-      // Propagate conforms_to attribute from public base classes.
-      for (auto base : clangDecl->bases()) {
-        if (base.getAccessSpecifier() != clang::AccessSpecifier::AS_public)
-          continue;
-        if (auto *baseClangDecl = base.getType()->getAsCXXRecordDecl())
-          addExplicitProtocolConformances(decl, baseClangDecl);
+      if (clangDecl->isCompleteDefinition()) {
+        // Propagate conforms_to attribute from public base classes.
+        for (auto base : clangDecl->bases()) {
+          if (base.getAccessSpecifier() != clang::AccessSpecifier::AS_public)
+            continue;
+          if (auto *baseClangDecl = base.getType()->getAsCXXRecordDecl())
+            addExplicitProtocolConformances(decl, baseClangDecl);
+        }
       }
 
       if (!clangDecl->hasAttrs())
@@ -10261,7 +10283,8 @@ ClangImporter::Implementation::importDeclForDeclContext(
 DeclContext *
 ClangImporter::Implementation::importDeclContextOf(
   const clang::Decl *decl,
-  EffectiveClangContext context)
+  EffectiveClangContext context,
+  bool allowForwardDeclaration)
 {
   DeclContext *importedDC = nullptr;
   switch (context.getKind()) {
@@ -10290,7 +10313,7 @@ ClangImporter::Implementation::importDeclContextOf(
     }
 
     if (dc->isTranslationUnit()) {
-      if (auto *module = getClangModuleForDecl(decl))
+      if (auto *module = getClangModuleForDecl(decl, allowForwardDeclaration))
         return module;
       else
         return nullptr;
@@ -10554,7 +10577,9 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
   }
 
   // If this is a C++ record, look through the base classes too.
-  if (auto cxxRecord = dyn_cast<clang::CXXRecordDecl>(clangRecord)) {
+  const clang::CXXRecordDecl *cxxRecord;
+  if ((cxxRecord = dyn_cast<clang::CXXRecordDecl>(clangRecord)) &&
+      cxxRecord->isCompleteDefinition()) {
     for (auto base : cxxRecord->bases()) {
       if (skipIfNonPublic && base.getAccessSpecifier() != clang::AS_public)
         continue;

lib/ClangImporter/ImporterImpl.h

@@ -1227,7 +1227,8 @@ public:
   /// \returns The imported declaration context, or null if it could not
   /// be converted.
   DeclContext *importDeclContextOf(const clang::Decl *D,
-                                   EffectiveClangContext context);
+                                   EffectiveClangContext context,
+                                   bool allowForwardDeclaration = false);
 
   /// Determine whether the given declaration is considered
   /// 'unavailable' in Swift.

lib/ClangImporter/SwiftDeclSynthesizer.cpp

@@ -2560,7 +2560,7 @@ llvm::SmallVector<clang::CXXMethodDecl *, 4>
 SwiftDeclSynthesizer::synthesizeStaticFactoryForCXXForeignRef(
     const clang::CXXRecordDecl *cxxRecordDecl) {
 
-  if (cxxRecordDecl->isAbstract())
+  if (!cxxRecordDecl->isCompleteDefinition() || cxxRecordDecl->isAbstract())
     return {};
 
   clang::ASTContext &clangCtx = cxxRecordDecl->getASTContext();

lib/ClangImporter/SwiftLookupTable.cpp

@@ -1881,19 +1881,9 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
   if (shouldSuppressDeclImport(named))
     return;
 
-  // Leave incomplete struct/enum/union types out of the table; Swift only
-  // handles pointers to them.
-  // FIXME: At some point we probably want to be importing incomplete types,
-  // so that pointers to different incomplete types themselves have distinct
-  // types. At that time it will be necessary to make the decision of whether
-  // or not to import an incomplete type declaration based on whether it's
-  // actually the struct backing a CF type:
-  //
-  //    typedef struct CGColor *CGColorRef;
-  //
-  // The best way to do this is probably to change CFDatabase.def to include
-  // struct names when relevant, not just pointer names. That way we can check
-  // both CFDatabase.def and the objc_bridge attribute and cover all our bases.
+  // Leave incomplete struct/enum/union types out of the table, unless they
+  // are types that will be imported as reference types (e.g., CF types or
+  // those that use SWIFT_SHARED_REFERENCE).
   if (auto *tagDecl = dyn_cast<clang::TagDecl>(named)) {
     // We add entries for ClassTemplateSpecializations that don't have
     // definition. It's possible that the decl will be instantiated by
@@ -1901,7 +1891,9 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
     // ClassTemplateSPecializations here because we're currently writing the
     // AST, so we cannot modify it.
     if (!isa<clang::ClassTemplateSpecializationDecl>(named) &&
-        !tagDecl->getDefinition()) {
+        !tagDecl->getDefinition() &&
+        !(isa<clang::RecordDecl>(tagDecl) &&
+          hasImportAsRefAttr(cast<clang::RecordDecl>(tagDecl)))) {
       return;
     }
   }

test/Interop/Cxx/foreign-reference/Inputs/ReferenceCounted.apinotes

@@ -0,0 +1,8 @@
+---
+Name: ReferenceCounted
+Tags:
+- Name: OpaqueRefImpl
+  SwiftImportAs: reference
+  SwiftRetainOp: OPRetain
+  SwiftReleaseOp: OPRelease
+

test/Interop/Cxx/foreign-reference/Inputs/incomplete.c

@@ -0,0 +1,55 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "reference-counted.h"
+
+typedef struct IncompleteImpl {
+  unsigned refCount;
+  double weight;
+} *Incomplete;
+
+
+Incomplete Incomplete_create(double weight) {
+  Incomplete result = (Incomplete)malloc(sizeof(struct IncompleteImpl));
+  result->refCount = 1;
+  result->weight = weight;
+  return result;
+}
+
+void INRetain(Incomplete i) {
+  i->refCount++;
+}
+
+void INRelease(Incomplete i) {
+  i->refCount--;
+  if (i->refCount == 0) {
+    printf("Destroyed instance containing weight %f\n", i->weight);
+    free(i);
+  }
+}
+
+double Incomplete_getWeight(Incomplete i) {
+  return i->weight;
+}
+
+struct OpaqueRefImpl {
+  unsigned refCount;
+};
+
+OpaqueRef Opaque_create(void) {
+  OpaqueRef result = (OpaqueRef)malloc(sizeof(struct OpaqueRefImpl));
+  result->refCount = 1;
+  return result;
+}
+
+void OPRetain(OpaqueRef i) {
+  i->refCount++;
+}
+
+void OPRelease(OpaqueRef i) {
+  i->refCount--;
+  if (i->refCount == 0) {
+    printf("Destroyed OpaqueRef instance\n");
+    free(i);
+  }
+}

test/Interop/Cxx/foreign-reference/Inputs/module.modulemap

@@ -35,7 +35,6 @@ module WitnessTable {
 
 module ReferenceCounted {
   header "reference-counted.h"
-  requires cplusplus
 }
 
 module ReferenceCountedObjCProperty {

test/Interop/Cxx/foreign-reference/Inputs/reference-counted.h

@@ -2,7 +2,10 @@
 #define TEST_INTEROP_CXX_FOREIGN_REFERENCE_INPUTS_REFERENCE_COUNTED_H
 
 #include <stdlib.h>
+
+#ifdef __cplusplus
 #include <new>
+#endif
 
 #include "visibility.h"
 
@@ -10,6 +13,8 @@ SWIFT_BEGIN_NULLABILITY_ANNOTATIONS
 
 static int finalLocalRefCount = 100;
 
+#ifdef __cplusplus
+
 namespace NS {
 
 struct __attribute__((swift_attr("import_as_ref")))
@@ -63,6 +68,30 @@ GlobalCountNullableInit {
 
 inline void GCRetainNullableInit(GlobalCountNullableInit *x) { globalCount++; }
 inline void GCReleaseNullableInit(GlobalCountNullableInit *x) { globalCount--; }
+#endif
+
+typedef struct __attribute__((swift_attr("import_as_ref")))
+__attribute__((swift_attr("retain:INRetain")))
+__attribute__((swift_attr("release:INRelease"))) IncompleteImpl *Incomplete;
+
+typedef struct OpaqueRefImpl *OpaqueRef;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+Incomplete Incomplete_create(double weight) __attribute__((swift_attr("returns_retained"))) __attribute__((swift_name("IncompleteImpl.init(weight:)")));
+void INRetain(Incomplete i);
+void INRelease(Incomplete i);
+double Incomplete_getWeight(Incomplete i) __attribute__((swift_name("getter:IncompleteImpl.weight(self:)")));
+
+OpaqueRef Opaque_create(void);
+void OPRetain(OpaqueRef i);
+void OPRelease(OpaqueRef i);
+
+#ifdef __cplusplus
+}
+#endif
 
 SWIFT_END_NULLABILITY_ANNOTATIONS
 

test/Interop/Cxx/foreign-reference/incomplete.swift

@@ -0,0 +1,47 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-clang -x c -c %S/Inputs/incomplete.c -I %S/Inputs -o %t/incomplete-c.o
+
+// Build with C interoperatibility
+// RUN: %target-build-swift %s -I %S/Inputs -o %t/incomplete %t/incomplete-c.o
+// RUN: %target-codesign %t/incomplete
+// RUN: %target-run %t/incomplete | %FileCheck %s
+
+// Build with C++ interoperability
+// RUN: %target-build-swift %s -I %S/Inputs -o %t/incomplete %t/incomplete-c.o -Xfrontend -cxx-interoperability-mode=default
+// RUN: %target-codesign %t/incomplete
+// RUN: %target-run %t/incomplete | %FileCheck %s
+
+// REQUIRES: executable_test
+
+import ReferenceCounted
+
+func testIncomplete() {
+  do {
+    let i = Incomplete(weight: 3.14159)
+    // CHECK: Incomplete weight = 3.14159
+    print("Incomplete weight = \(i.weight)")
+
+    // Instance destroyed at the end
+  }
+
+  // CHECK: Destroyed instance containing weight 3.14159
+}
+
+func testOpaqueRef() {
+  // CHECK: Creating OpaqueRef
+  print("Creating OpaqueRef")
+  let opaque = Opaque_create()
+  let opaque2 = opaque
+  _ = opaque
+  _ = opaque2
+  // let it go out of scope
+
+  // CHECK: Destroyed OpaqueRef instance
+  // CHECK-NOT: Destroyed OpaqueRef instance
+}
+
+testIncomplete()
+testOpaqueRef()
+
+// CHECK: DONE
+print("DONE")