diff --git a/lib/ClangImporter/SwiftifyDecl.cpp b/lib/ClangImporter/SwiftifyDecl.cpp index 9a7770117092f..5ceacfce7ee10 100644 --- a/lib/ClangImporter/SwiftifyDecl.cpp +++ b/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(D)) { + return FuncD->getTemplateInstantiationPattern(); + } + if (auto RecordD = dyn_cast(D)) { + return RecordD->getTemplateInstantiationPattern(); + } + if (auto EnumD = dyn_cast(D)) { + return EnumD->getTemplateInstantiationPattern(); + } + if (auto VarD = dyn_cast(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(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(); @@ -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(); if (CAT && mappedIndex == SwiftifyInfoPrinter::SELF_PARAM_INDEX) { @@ -558,7 +657,7 @@ void ClangImporter::Implementation::swiftify(AbstractFunctionDecl *MappedDecl) { MacroDecl *SwiftifyImportDecl = dyn_cast_or_null(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( getKnownSingleDecl(SwiftContext, "_SwiftifyImportProtocol")); if (!SwiftifyImportDecl) { - DLOG("_SwiftifyImportProtocol macro not found"); + DLOG("_SwiftifyImportProtocol macro not found\n"); return; } diff --git a/test/Interop/C/swiftify-import/bridging-header-from-module.swift b/test/Interop/C/swiftify-import/bridging-header-from-module.swift new file mode 100644 index 0000000000000..a6f0a7adf5c5f --- /dev/null +++ b/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, _ x: UnsafeMutablePointer!) { + 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, x: UnsafeMutablePointer) { + 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 +#include +// 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" +} diff --git a/test/Interop/C/swiftify-import/bridging-header.swift b/test/Interop/C/swiftify-import/bridging-header.swift index 4124fbb12b52c..bf7c9667f7f7a 100644 --- a/test/Interop/C/swiftify-import/bridging-header.swift +++ b/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) { - 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, _ x: UnsafeMutablePointer!) { + 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) { - foo(s) +import A +import B +import C + +func test(s: Span, x: UnsafeMutablePointer) { + unsafe foo(s, x) +} + +func test2(p: UnsafeMutablePointer, len: CInt, y: UnsafeMutablePointer) { + unsafe bar(len, p, y) +} + +func test3(p: UnsafeMutablePointer, len: CInt, z: UnsafeMutablePointer) { + unsafe baz(len, p, z) } //--- bridging.h #include #include -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" +} 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..6561bd1e6bf65 --- /dev/null +++ b/test/Interop/C/swiftify-import/conflicting-opaqueness.swift @@ -0,0 +1,171 @@ +// 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{{'fooIndirect' declared here}} +void fooIndirect(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); + +struct container_t { + struct qux *item; +}; +// expected-expansion@+10:6{{ +// expected-note@1 5{{in expansion of macro '_SwiftifyImport' on global function 'fooWrapped' here}} +// }} +// expected-expansion@+7:75{{ +// expected-remark@2{{macro content: |@_alwaysEmitIntoClient @_disfavoredOverload public func fooWrapped(_ x: UnsafeMutablePointer!, _ p: UnsafeMutableBufferPointer) {|}} +// expected-remark@3{{macro content: | let len = Int32(exactly: p.count)!|}} +// expected-remark@4{{macro content: | return unsafe fooWrapped(x, p.baseAddress!, len)|}} +// expected-remark@5{{macro content: |}|}} +// expected-remark@1{{macro content: |/// This is an auto-generated wrapper for safer interop|}} +// }} +void fooWrapped(struct container_t * x, 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, _ y: UnsafeMutablePointer?>, _ z: UnsafeMutablePointer, _ p: UnsafeMutablePointer, _ len: CInt) { + unsafe foo(x, p, len) + unsafe fooIndirect(y, p, len) + let _: UnsafeMutablePointer = unsafe fooReturn(p, len) + unsafe fooWrapped(z, p, len) +} + +func callFoo2(_ x: UnsafeMutablePointer, _ y: UnsafeMutablePointer?>, _ z: 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 #3 in call}} + // expected-error@+1{{cannot convert value of type 'UnsafeMutableBufferPointer' (aka 'UnsafeMutableBufferPointer') to expected argument type 'UnsafeMutablePointer'}} + unsafe fooIndirect(y, 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) + unsafe fooWrapped(z, 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" +}