[Swiftify] don't add macro when required type is unavailable

lib/ClangImporter/SwiftifyDecl.cpp

@@ -20,12 +20,17 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/DiagnosticsClangImporter.h"
 #include "swift/AST/ParameterList.h"
+#include "swift/AST/TypeWalker.h"
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/Module.h"
 
 using namespace swift;
 using namespace importer;
@@ -225,7 +230,7 @@ struct CountedByExpressionValidator
     case clang::BuiltinType::ULong:
     case clang::BuiltinType::LongLong:
     case clang::BuiltinType::ULongLong:
-      DLOG("Ignoring count parameter with non-portable integer literal");
+      DLOG("Ignoring count parameter with non-portable integer literal\n");
       return false;
     default:
       return true;
@@ -271,14 +276,17 @@ struct CountedByExpressionValidator
 };
 
 
-// Don't try to transform any Swift types that _SwiftifyImport doesn't know how
-// to handle.
-static bool SwiftifiableCountedByPointerType(Type swiftType) {
+static Type ConcretePointeeType(Type swiftType) {
   Type nonnullType = swiftType->lookThroughSingleOptionalType();
   PointerTypeKind PTK;
-  return nonnullType->getAnyPointerElementType(PTK) &&
-    (PTK == PTK_UnsafePointer || PTK == PTK_UnsafeMutablePointer);
+  Type PointeeTy = nonnullType->getAnyPointerElementType(PTK);
+  if (PointeeTy && (PTK == PTK_UnsafePointer || PTK == PTK_UnsafeMutablePointer))
+    return PointeeTy;
+  return Type();
 }
+
+// Don't try to transform any Swift types that _SwiftifyImport doesn't know how
+// to handle.
 static bool SwiftifiableSizedByPointerType(const clang::ASTContext &ctx,
                                            Type swiftType,
                                            const clang::CountAttributedType *CAT) {
@@ -311,7 +319,7 @@ static bool SwiftifiableCAT(const clang::ASTContext &ctx,
   return CAT && CountedByExpressionValidator().Visit(CAT->getCountExpr()) &&
     (CAT->isCountInBytes() ?
        SwiftifiableSizedByPointerType(ctx, swiftType, CAT)
-     : SwiftifiableCountedByPointerType(swiftType));
+     : !ConcretePointeeType(swiftType).isNull());
 }
 
 // Searches for template instantiations that are not behind type aliases.
@@ -326,11 +334,84 @@ struct UnaliasedInstantiationVisitor
   bool
   VisitTemplateSpecializationType(const clang::TemplateSpecializationType *) {
     hasUnaliasedInstantiation = true;
-    DLOG("Signature contains raw template, skipping");
+    DLOG("Signature contains raw template, skipping\n");
     return false;
   }
 };
 
+static const clang::Decl *getTemplateInstantiation(const clang::Decl *D) {
+  if (auto FuncD = dyn_cast<clang::FunctionDecl>(D)) {
+    return FuncD->getTemplateInstantiationPattern();
+  }
+  if (auto RecordD = dyn_cast<clang::CXXRecordDecl>(D)) {
+    return RecordD->getTemplateInstantiationPattern();
+  }
+  if (auto EnumD = dyn_cast<clang::EnumDecl>(D)) {
+    return EnumD->getTemplateInstantiationPattern();
+  }
+  if (auto VarD = dyn_cast<clang::VarDecl>(D)) {
+    return VarD->getTemplateInstantiationPattern();
+  }
+  return nullptr;
+}
+
+static clang::Module *getOwningModule(const clang::Decl *ClangDecl) {
+  if (const auto *Instance = getTemplateInstantiation(ClangDecl)) {
+    return Instance->getOwningModule();
+  }
+  return ClangDecl->getOwningModule();
+}
+
+struct ForwardDeclaredConcreteTypeVisitor : public TypeWalker {
+  bool hasForwardDeclaredConcreteType = false;
+  const clang::Module *Owner;
+
+  ForwardDeclaredConcreteTypeVisitor(const clang::Module *Owner) : Owner(Owner) {};
+
+  Action walkToTypePre(Type ty) override {
+    DLOG("Walking type:\n");
+    LLVM_DEBUG(DUMP(ty));
+    if (Type PointeeTy = ConcretePointeeType(ty)) {
+      auto *Nom = PointeeTy->getAnyNominal();
+      const clang::Decl *ClangDecl = Nom->getClangDecl();
+      if (!ClangDecl) {
+        return Action::Continue;
+      }
+
+      if (auto RD = dyn_cast<clang::RecordDecl>(ClangDecl)) {
+        const clang::RecordDecl *Def = RD->getDefinition();
+        ASSERT(Def && "pointer to concrete type without type definition?");
+        const clang::Module *M = getOwningModule(ClangDecl);
+
+        if (!M) {
+          DLOG("Concrete type is in bridging header, which is always imported\n");
+          return Action::Continue;
+        }
+
+        if (!Owner) {
+          hasForwardDeclaredConcreteType = true;
+          DLOG("Imported signature contains concrete type not available in bridging header, skipping\n");
+          LLVM_DEBUG(DUMP(Def));
+          return Action::Stop;
+        }
+        if (!Owner->isModuleVisible(M)) {
+          hasForwardDeclaredConcreteType = true;
+          DLOG("Imported signature contains concrete type not available in clang module, skipping\n");
+          LLVM_DEBUG(DUMP(Def));
+          return Action::Stop;
+        }
+      }
+    }
+    return Action::Continue;
+  }
+
+  bool IsIncompatibleImport(Type SwiftTy, clang::QualType ClangTy) {
+    DLOG_SCOPE("Checking compatibility of type: " << ClangTy << "\n");
+    SwiftTy.walk(*this);
+    return hasForwardDeclaredConcreteType;
+  }
+};
+
 // until CountAttributedType::getAttributeName lands in our LLVM branch
 static StringRef getAttributeName(const clang::CountAttributedType *CAT) {
   switch (CAT->getKind()) {
@@ -373,8 +454,18 @@ static bool swiftifyImpl(ClangImporter::Implementation &Self,
       ClangDecl->getAccess() == clang::AS_private)
     return false;
 
+  if (ClangDecl->isImplicit()) {
+    DLOG("implicit functions lack lifetime and bounds info\n");
+    return false;
+  }
+
   clang::ASTContext &clangASTContext = Self.getClangASTContext();
 
+  const clang::Module *OwningModule = getOwningModule(ClangDecl);
+  bool IsInBridgingHeader = MappedDecl->getModuleContext()->getName().str() == CLANG_HEADER_MODULE_NAME;
+  ASSERT(OwningModule || IsInBridgingHeader);
+  ForwardDeclaredConcreteTypeVisitor CheckForwardDecls(OwningModule);
+
   // We only attach the macro if it will produce an overload. Any __counted_by
   // will produce an overload, since UnsafeBufferPointer is still an improvement
   // over UnsafePointer, but std::span will only produce an overload if it also
@@ -389,6 +480,10 @@ static bool swiftifyImpl(ClangImporter::Implementation &Self,
     else
       ABORT("Unexpected AbstractFunctionDecl subclass.");
     clang::QualType clangReturnTy = ClangDecl->getReturnType();
+
+    if (CheckForwardDecls.IsIncompatibleImport(swiftReturnTy, clangReturnTy))
+      return false;
+
     bool returnIsStdSpan = printer.registerStdSpanTypeMapping(
         swiftReturnTy, clangReturnTy);
     auto *CAT = clangReturnTy->getAs<clang::CountAttributedType>();
@@ -440,6 +535,10 @@ static bool swiftifyImpl(ClangImporter::Implementation &Self,
       }
       ASSERT(swiftParam);
       Type swiftParamTy = swiftParam->getInterfaceType();
+
+      if (CheckForwardDecls.IsIncompatibleImport(swiftParamTy, clangParamTy))
+        return false;
+
       bool paramHasBoundsInfo = false;
       auto *CAT = clangParamTy->getAs<clang::CountAttributedType>();
       if (CAT && mappedIndex == SwiftifyInfoPrinter::SELF_PARAM_INDEX) {
@@ -558,7 +657,7 @@ void ClangImporter::Implementation::swiftify(AbstractFunctionDecl *MappedDecl) {
 
   MacroDecl *SwiftifyImportDecl = dyn_cast_or_null<MacroDecl>(getKnownSingleDecl(SwiftContext, "_SwiftifyImport"));
   if (!SwiftifyImportDecl) {
-    DLOG("_SwiftifyImport macro not found");
+    DLOG("_SwiftifyImport macro not found\n");
     return;
   }
 
@@ -610,7 +709,7 @@ void ClangImporter::Implementation::swiftifyProtocol(
   MacroDecl *SwiftifyImportDecl = dyn_cast_or_null<MacroDecl>(
       getKnownSingleDecl(SwiftContext, "_SwiftifyImportProtocol"));
   if (!SwiftifyImportDecl) {
-    DLOG("_SwiftifyImportProtocol macro not found");
+    DLOG("_SwiftifyImportProtocol macro not found\n");
     return;
   }
 

test/Interop/C/swiftify-import/bridging-header-from-module.swift

@@ -0,0 +1,71 @@
+// REQUIRES: swift_feature_SafeInteropWrappers
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -typecheck -plugin-path %swift-plugin-dir -o %t/test.swiftmodule -I %t -import-objc-header %t/bridging.h -enable-experimental-feature SafeInteropWrappers -strict-memory-safety -warnings-as-errors -Xcc -Werror -Xcc -Wno-nullability-completeness -Xcc -Wno-div-by-zero -Xcc -Wno-pointer-to-int-cast %t/test.swift -verify
+// RUN: %target-swift-frontend -typecheck -plugin-path %swift-plugin-dir -o %t/test.swiftmodule -I %t -import-objc-header %t/bridging.h -enable-experimental-feature SafeInteropWrappers -strict-memory-safety -warnings-as-errors -Xcc -Werror -Xcc -Wno-nullability-completeness -Xcc -Wno-div-by-zero -Xcc -Wno-pointer-to-int-cast %t/test.swift -dump-macro-expansions 2>&1 | %FileCheck --dry-run > %t/macro-expansions.out
+// RUN: %diff %t/macro-expansions.out %t/macro-expansions.expected
+// RUN: %target-swift-frontend -typecheck -plugin-path %swift-plugin-dir -o %t/test.swiftmodule -I %t -import-objc-header %t/bridging.h -enable-experimental-feature SafeInteropWrappers -strict-memory-safety -warnings-as-errors -Xcc -Werror -Xcc -Wno-nullability-completeness -Xcc -Wno-div-by-zero -Xcc -Wno-pointer-to-int-cast %t/test.swift -dump-source-file-imports 2>&1 | %FileCheck --dry-run > %t/imports.out
+// RUN: %diff %t/imports.out %t/imports.expected
+
+//--- imports.expected
+imports for TMP_DIR/test.swift:
+	Swift
+	__ObjC
+	_StringProcessing
+	_SwiftConcurrencyShims
+	_Concurrency
+	A
+imports for A.foo:
+imports for @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:
+	Swift
+	__ObjC
+	lifetimebound
+	ptrcheck
+	_StringProcessing
+	_SwiftConcurrencyShims
+	_Concurrency
+
+//--- macro-expansions.expected
+@__swiftmacro_So3foo15_SwiftifyImportfMp_.swift
+------------------------------
+/// This is an auto-generated wrapper for safer interop
+@_alwaysEmitIntoClient @available(visionOS 1.0, tvOS 12.2, watchOS 5.2, iOS 12.2, macOS 10.14.4, *) @_disfavoredOverload public func foo(_ p: Span<Int32>, _ x: UnsafeMutablePointer<no_module_record_t>!) {
+    let len = Int32(exactly: p.count)!
+    return unsafe p.withUnsafeBufferPointer { _pPtr in
+      return unsafe foo(len, _pPtr.baseAddress!, x)
+    }
+}
+------------------------------
+
+//--- test.swift
+import A
+
+func test(s: Span<a_t>, x: UnsafeMutablePointer<no_module_record_t>) {
+  unsafe foo(s, x)
+}
+
+//--- bridging.h
+// claim this header as part of bridging header, making it not belong to a module
+#include "no-module.h"
+
+//--- no-module.h
+#pragma once
+typedef int no_module_t;
+struct no_module_record_t {
+  int placeholder;
+};
+
+//--- a.h
+#include <ptrcheck.h>
+#include <lifetimebound.h>
+// this module header now depends on a decl with no module
+#include "no-module.h"
+typedef int a_t;
+void foo(int len, const int * __counted_by(len) p __noescape, struct no_module_record_t *x);
+
+//--- module.modulemap
+module A {
+  header "a.h"
+}

test/Interop/C/swiftify-import/bridging-header.swift

@@ -16,6 +16,9 @@ imports for TMP_DIR/test.swift:
 	_StringProcessing
 	_SwiftConcurrencyShims
 	_Concurrency
+	A
+	B
+	C
 imports for __ObjC.foo:
 imports for @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:
 	__ObjC
@@ -25,21 +28,73 @@ imports for @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift:
 @__swiftmacro_So3foo15_SwiftifyImportfMp_.swift
 ------------------------------
 /// This is an auto-generated wrapper for safer interop
-@_alwaysEmitIntoClient @available(visionOS 1.0, tvOS 12.2, watchOS 5.2, iOS 12.2, macOS 10.14.4, *) @_disfavoredOverload public func foo(_ p: Span<Int32>) {
-    let len = Int32(exactly: p.count)!
+@_alwaysEmitIntoClient @available(visionOS 1.0, tvOS 12.2, watchOS 5.2, iOS 12.2, macOS 10.14.4, *) @_disfavoredOverload public func foo(_ p: Span<a_t>, _ x: UnsafeMutablePointer<no_module_record_t>!) {
+    let len = no_module_t(exactly: p.count)!
     return unsafe p.withUnsafeBufferPointer { _pPtr in
-      return unsafe foo(len, _pPtr.baseAddress!)
+      return unsafe foo(len, _pPtr.baseAddress!, x)
     }
 }
 ------------------------------
 
 //--- test.swift
-func test(s: Span<CInt>) {
-  foo(s)
+import A
+import B
+import C
+
+func test(s: Span<a_t>, x: UnsafeMutablePointer<no_module_record_t>) {
+  unsafe foo(s, x)
+}
+
+func test2(p: UnsafeMutablePointer<CInt>, len: CInt, y: UnsafeMutablePointer<b_t>) {
+  unsafe bar(len, p, y)
+}
+
+func test3(p: UnsafeMutablePointer<CInt>, len: CInt, z: UnsafeMutablePointer<c_t>) {
+  unsafe baz(len, p, z)
 }
 
 //--- bridging.h
 #include <ptrcheck.h>
 #include <lifetimebound.h>
 
-void foo(int len, const int * __counted_by(len) p __noescape);
+#include "no-module.h"
+#include "a.h"
+#include "c.h"
+
+struct no_module_record_t;
+void foo(no_module_t len, const a_t * __counted_by(len) p __noescape, struct no_module_record_t *x);
+
+struct b_t;
+void bar(int len, const int * __counted_by(len) p __noescape, struct b_t *y);
+
+void baz(int len, const int * __counted_by(len) p __noescape, struct c_t *z);
+
+//--- no-module.h
+typedef int no_module_t;
+struct no_module_record_t {
+  int placeholder;
+};
+
+//--- a.h
+typedef int a_t;
+
+//--- b.h
+struct b_t {
+  int placeholder;
+};
+
+//--- c.h
+struct c_t {
+  int placeholder;
+};
+
+//--- module.modulemap
+module A {
+  header "a.h"
+}
+module B {
+  header "b.h"
+}
+module C {
+  header "c.h"
+}