diff --git a/lib/ClangImporter/SwiftifyDecl.cpp b/lib/ClangImporter/SwiftifyDecl.cpp index 9a7770117092f..54c59c177cad7 100644 --- a/lib/ClangImporter/SwiftifyDecl.cpp +++ b/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 { + 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 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(); @@ -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(); if (CAT && mappedIndex == SwiftifyInfoPrinter::SELF_PARAM_INDEX) { diff --git a/test/Interop/C/swiftify-import/conflicting-opaqueness.swift b/test/Interop/C/swiftify-import/conflicting-opaqueness.swift new file mode 100644 index 0000000000000..a000c541e74d1 --- /dev/null +++ b/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!, _ p: UnsafeMutableBufferPointer) {|}} +// 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) -> UnsafeMutablePointer! {|}} +// 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!, _ p: UnsafeMutableBufferPointer) {|}} +// 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) -> UnsafeMutablePointer! {|}} +// 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, _ p: UnsafeMutablePointer, _ len: CInt) { + unsafe foo(x, p, len) + let _: UnsafeMutablePointer = unsafe fooReturn(p, len) +} + +func callFoo2(_ x: UnsafeMutablePointer, _ p: UnsafeMutableBufferPointer) { + // expected-error@+2{{missing argument for parameter #3 in call}} + // expected-error@+1{{cannot convert value of type 'UnsafeMutableBufferPointer' (aka 'UnsafeMutableBufferPointer') to expected argument type 'UnsafeMutablePointer'}} + unsafe foo(x, p) + // expected-error@+2{{missing argument for parameter #2 in call}} + // expected-error@+1{{cannot convert value of type 'UnsafeMutableBufferPointer' (aka 'UnsafeMutableBufferPointer') to expected argument type 'UnsafeMutablePointer'}} + let _: UnsafeMutablePointer = unsafe fooReturn(p) +} + + +func callBar(_ x: UnsafeMutablePointer, _ p: UnsafeMutablePointer, _ len: CInt) { + unsafe bar(x, p, len) + let _: UnsafeMutablePointer = unsafe barReturn(p, len) +} + +func callBar2(_ x: UnsafeMutablePointer, _ p: UnsafeMutableBufferPointer) { + unsafe bar(x, p) + let _: UnsafeMutablePointer = unsafe barReturn(p) +} + + +func callBaz(_ x: UnsafeMutablePointer, _ p: UnsafeMutablePointer, _ len: CInt) { + unsafe baz(x, p, len) + let _: UnsafeMutablePointer = unsafe bazReturn(p, len) +} + +func callBaz2(_ x: UnsafeMutablePointer, _ p: UnsafeMutableBufferPointer) { + unsafe baz(x, p) + let _: UnsafeMutablePointer = 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" +}