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

lib/ClangImporter/SwiftifyDecl.cpp

@@ -24,8 +24,11 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.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;
@@ -271,14 +274,15 @@ struct CountedByExpressionValidator
 };
 
 
-// Don't try to transform any Swift types that _SwiftifyImport doesn't know how
-// to handle.
-static bool SwiftifiableCountedByPointerType(Type swiftType) {
+static bool IsConcretePointerType(Type swiftType) {
   Type nonnullType = swiftType->lookThroughSingleOptionalType();
   PointerTypeKind PTK;
   return nonnullType->getAnyPointerElementType(PTK) &&
     (PTK == PTK_UnsafePointer || PTK == PTK_UnsafeMutablePointer);
 }
+
+// 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 +315,7 @@ static bool SwiftifiableCAT(const clang::ASTContext &ctx,
   return CAT && CountedByExpressionValidator().Visit(CAT->getCountExpr()) &&
     (CAT->isCountInBytes() ?
        SwiftifiableSizedByPointerType(ctx, swiftType, CAT)
-     : SwiftifiableCountedByPointerType(swiftType));
+     : IsConcretePointerType(swiftType));
 }
 
 // Searches for template instantiations that are not behind type aliases.
@@ -331,6 +335,36 @@ struct UnaliasedInstantiationVisitor
   }
 };
 
+struct ForwardDeclaredConcreteTypeVisitor
+    : clang::RecursiveASTVisitor<ForwardDeclaredConcreteTypeVisitor> {
+  bool hasForwardDeclaredConcreteType = false;
+  const clang::Module *Owner;
+
+  ForwardDeclaredConcreteTypeVisitor(const clang::Module *Owner) : Owner(Owner) { ASSERT(Owner); };
+
+  bool VisitRecordType(clang::RecordType *RT) {
+    const clang::RecordDecl *RD = RT->getDecl()->getDefinition();
+    ASSERT(RD && "pointer to concrete type without type definition?");
+    const clang::Module *M = RD->getOwningModule();
+    if (!Owner->isModuleVisible(M)) {
+      hasForwardDeclaredConcreteType = true;
+      DLOG("Imported signature contains concrete type not available in clang module, skipping");
+      LLVM_DEBUG(DUMP(RD));
+      return false;
+    }
+    return true;
+  }
+
+  bool IsIncompatibleImport(Type SwiftTy, clang::QualType ClangTy) {
+    DLOG_SCOPE("Checking compatibility of type: " << ClangTy << "\n");
+    if (!IsConcretePointerType(SwiftTy)) {
+      return false;
+    }
+    TraverseType(ClangTy);
+    return hasForwardDeclaredConcreteType;
+  }
+};
+
 // until CountAttributedType::getAttributeName lands in our LLVM branch
 static StringRef getAttributeName(const clang::CountAttributedType *CAT) {
   switch (CAT->getKind()) {
@@ -360,6 +394,13 @@ static size_t getNumParams(const clang::FunctionDecl* D) {
 }
 } // namespace
 
+static const clang::Decl *getTemplateInstantiation(const clang::FunctionDecl *D) {
+  return D->getTemplateInstantiationPattern();
+}
+static const clang::Decl *getTemplateInstantiation(const clang::ObjCMethodDecl *) {
+  return nullptr;
+}
+
 template<typename T>
 static bool swiftifyImpl(ClangImporter::Implementation &Self,
                          SwiftifyInfoFunctionPrinter &printer,
@@ -373,8 +414,21 @@ static bool swiftifyImpl(ClangImporter::Implementation &Self,
       ClangDecl->getAccess() == clang::AS_private)
     return false;
 
+  if (ClangDecl->isImplicit()) {
+    DLOG("implicit functions lack lifetime and bounds info");
+    return false;
+  }
+
   clang::ASTContext &clangASTContext = Self.getClangASTContext();
 
+  const clang::Module *OwningModule = nullptr;
+  if (const auto *Instance = getTemplateInstantiation(ClangDecl)) {
+    OwningModule = Instance->getOwningModule();
+  } else {
+    OwningModule = ClangDecl->getOwningModule();
+  }
+  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 +443,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 +498,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) {

test/Interop/C/swiftify-import/conflicting-opaqueness.swift

@@ -0,0 +1,148 @@
+// REQUIRES: swift_feature_SafeInteropWrappers
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/test.swiftmodule %t/test.swift -I %t -enable-experimental-feature SafeInteropWrappers -strict-memory-safety \
+// RUN:   -verify -verify-additional-file %t%{fs-sep}foo.h -verify-additional-file %t%{fs-sep}bar.h -verify-additional-file %t%{fs-sep}baz.h -verify-additional-file %t%{fs-sep}qux.h -Rmacro-expansions
+
+// Macro expansions in foo.h do not have access to the definition of `struct qux`,
+// so don't attach macro on functions that use contain that type in foo.h.
+// bar.h imports the type, so attach the macro.
+// baz.h imports bar.h which re-exports the type, so attach the macro there too.
+
+//--- foo.h
+#pragma once
+
+#define __counted_by(x) __attribute__((__counted_by__(x)))
+
+struct qux;
+// expected-note@+1{{'foo' declared here}}
+void foo(struct qux *x, int * __counted_by(len) p, int len);
+// expected-note@+1{{'fooReturn' declared here}}
+struct qux * fooReturn(int * __counted_by(len) p, int len);
+
+
+//--- bar.h
+#pragma once
+#include "qux.h"
+
+#define __counted_by(x) __attribute__((__counted_by__(x)))
+
+// expected-expansion@+10:59{{
+//   expected-remark@1{{macro content: |/// This is an auto-generated wrapper for safer interop|}}
+//   expected-remark@2{{macro content: |@_alwaysEmitIntoClient @_disfavoredOverload public func bar(_ x: UnsafeMutablePointer<qux>!, _ p: UnsafeMutableBufferPointer<Int32>) {|}}
+//   expected-remark@3{{macro content: |    let len = Int32(exactly: p.count)!|}}
+//   expected-remark@4{{macro content: |    return unsafe bar(x, p.baseAddress!, len)|}}
+//   expected-remark@5{{macro content: |}|}}
+// }}
+// expected-expansion@+3:6{{
+//   expected-note@1 5{{in expansion of macro '_SwiftifyImport' on global function 'bar' here}}
+// }}
+void bar(struct qux *x, int * __counted_by(len) p, int len);
+// expected-expansion@+10:58{{
+//   expected-remark@1{{macro content: |/// This is an auto-generated wrapper for safer interop|}}
+//   expected-remark@2{{macro content: |@_alwaysEmitIntoClient @_disfavoredOverload public func barReturn(_ p: UnsafeMutableBufferPointer<Int32>) -> UnsafeMutablePointer<qux>! {|}}
+//   expected-remark@3{{macro content: |    let len = Int32(exactly: p.count)!|}}
+//   expected-remark@4{{macro content: |    return unsafe barReturn(p.baseAddress!, len)|}}
+//   expected-remark@5{{macro content: |}|}}
+// }}
+// expected-expansion@+3:14{{
+//   expected-note@1 5{{in expansion of macro '_SwiftifyImport' on global function 'barReturn' here}}
+// }}
+struct qux * barReturn(int * __counted_by(len) p, int len);
+
+
+//--- baz.h
+#pragma once
+
+#include "bar.h"
+
+struct qux;
+// expected-expansion@+10:59{{
+//   expected-remark@1{{macro content: |/// This is an auto-generated wrapper for safer interop|}}
+//   expected-remark@2{{macro content: |@_alwaysEmitIntoClient @_disfavoredOverload public func baz(_ x: UnsafeMutablePointer<qux>!, _ p: UnsafeMutableBufferPointer<Int32>) {|}}
+//   expected-remark@3{{macro content: |    let len = Int32(exactly: p.count)!|}}
+//   expected-remark@4{{macro content: |    return unsafe baz(x, p.baseAddress!, len)|}}
+//   expected-remark@5{{macro content: |}|}}
+// }}
+// expected-expansion@+3:6{{
+//   expected-note@1 5{{in expansion of macro '_SwiftifyImport' on global function 'baz' here}}
+// }}
+void baz(struct qux *x, int * __counted_by(len) p, int len);
+// expected-expansion@+10:58{{
+//   expected-remark@1{{macro content: |/// This is an auto-generated wrapper for safer interop|}}
+//   expected-remark@2{{macro content: |@_alwaysEmitIntoClient @_disfavoredOverload public func bazReturn(_ p: UnsafeMutableBufferPointer<Int32>) -> UnsafeMutablePointer<qux>! {|}}
+//   expected-remark@3{{macro content: |    let len = Int32(exactly: p.count)!|}}
+//   expected-remark@4{{macro content: |    return unsafe bazReturn(p.baseAddress!, len)|}}
+//   expected-remark@5{{macro content: |}|}}
+// }}
+// expected-expansion@+3:14{{
+//   expected-note@1 5{{in expansion of macro '_SwiftifyImport' on global function 'bazReturn' here}}
+// }}
+struct qux * bazReturn(int * __counted_by(len) p, int len);
+
+
+//--- qux.h
+#pragma once
+
+struct qux { int placeholder; };
+
+
+//--- test.swift
+import Foo
+import Bar
+import Baz
+
+func callFoo(_ x: UnsafeMutablePointer<qux>, _ p: UnsafeMutablePointer<CInt>, _ len: CInt) {
+  unsafe foo(x, p, len)
+  let _: UnsafeMutablePointer<qux> = unsafe fooReturn(p, len)
+}
+
+func callFoo2(_ x: UnsafeMutablePointer<qux>, _ p: UnsafeMutableBufferPointer<CInt>) {
+  // expected-error@+2{{missing argument for parameter #3 in call}}
+  // expected-error@+1{{cannot convert value of type 'UnsafeMutableBufferPointer<CInt>' (aka 'UnsafeMutableBufferPointer<Int32>') to expected argument type 'UnsafeMutablePointer<Int32>'}}
+  unsafe foo(x, p)
+  // expected-error@+2{{missing argument for parameter #2 in call}}
+  // expected-error@+1{{cannot convert value of type 'UnsafeMutableBufferPointer<CInt>' (aka 'UnsafeMutableBufferPointer<Int32>') to expected argument type 'UnsafeMutablePointer<Int32>'}}
+  let _: UnsafeMutablePointer<qux> = unsafe fooReturn(p)
+}
+
+
+func callBar(_ x: UnsafeMutablePointer<qux>, _ p: UnsafeMutablePointer<CInt>, _ len: CInt) {
+  unsafe bar(x, p, len)
+  let _: UnsafeMutablePointer<qux> = unsafe barReturn(p, len)
+}
+
+func callBar2(_ x: UnsafeMutablePointer<qux>, _ p: UnsafeMutableBufferPointer<CInt>) {
+  unsafe bar(x, p)
+  let _: UnsafeMutablePointer<qux> = unsafe barReturn(p)
+}
+
+
+func callBaz(_ x: UnsafeMutablePointer<qux>, _ p: UnsafeMutablePointer<CInt>, _ len: CInt) {
+  unsafe baz(x, p, len)
+  let _: UnsafeMutablePointer<qux> = unsafe bazReturn(p, len)
+}
+
+func callBaz2(_ x: UnsafeMutablePointer<qux>, _ p: UnsafeMutableBufferPointer<CInt>) {
+  unsafe baz(x, p)
+  let _: UnsafeMutablePointer<qux> = unsafe bazReturn(p)
+}
+
+
+//--- module.modulemap
+module Foo {
+  header "foo.h"
+}
+module Bar {
+  header "bar.h"
+  export qux
+}
+module Baz {
+  header "baz.h"
+  export Bar
+}
+module qux {
+  header "qux.h"
+}