Merge pull request #12729 from jrose-apple/bridging-typedefs-is-hard (#12737)
[ClangImporter] Bridging can happen even without full bridgeability
rdar://problem/34913634
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
index 6aacdc6..657fe4d 100644
--- a/lib/ClangImporter/ImportDecl.cpp
+++ b/lib/ClangImporter/ImportDecl.cpp
@@ -3457,6 +3457,8 @@
Impl.CurrentVersion))
declType = Impl.getClangASTContext().getTypedefType(newtypeDecl);
+ // Note that we deliberately don't bridge most globals because we want to
+ // preserve pointer identity.
Type type = Impl.importType(declType,
(isAudited ? ImportTypeKind::AuditedVariable
: ImportTypeKind::Variable),
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
index b7d318d..e8917a4 100644
--- a/lib/ClangImporter/ImportType.cpp
+++ b/lib/ClangImporter/ImportType.cpp
@@ -684,13 +684,18 @@
auto typedefBridgeability = getTypedefBridgeability(underlyingType);
// Figure out the typedef we should actually use.
- SwiftTypeConverter innerConverter(Impl, AllowNSUIntegerAsInt, Bridging);
+ auto underlyingBridgeability =
+ (Bridging == Bridgeability::Full
+ ? typedefBridgeability : Bridgeability::None);
+ SwiftTypeConverter innerConverter(Impl, AllowNSUIntegerAsInt,
+ underlyingBridgeability);
auto underlyingResult = innerConverter.Visit(underlyingType);
// If we used different bridgeability than this typedef normally
- // would, and therefore the underlying type is different from the
- // mapping of the typedef, use the underlying type.
- if (Bridging != typedefBridgeability &&
+ // would because we're in a non-bridgeable context, and therefore
+ // the underlying type is different from the mapping of the typedef,
+ // use the underlying type.
+ if (underlyingBridgeability != typedefBridgeability &&
!underlyingResult.AbstractType->isEqual(mappedType)) {
return underlyingResult;
}
@@ -1291,29 +1296,9 @@
// SwiftTypeConverter turns block pointers into @convention(block) types.
// In a bridgeable context, or in the direct structure of a typedef,
- // we would prefer to instead use the default Swift convention. But this
- // does means that, when we're using a typedef of a block pointer type in
- // an unbridgable context, we need to go back and do a fully-unbridged
- // import of the underlying type.
+ // we would prefer to instead use the default Swift convention.
if (hint == ImportHint::Block) {
- if (bridging == Bridgeability::None) {
- if (auto typedefType = clangType->getAs<clang::TypedefType>()) {
- // In non-bridged contexts, drop the typealias sugar for blocks.
- // FIXME: This will do the wrong thing if there's any adjustment to do
- // besides optionality.
- Type underlyingTy = impl.importType(typedefType->desugar(),
- importKind,
- allowNSUIntegerAsInt,
- bridging,
- OTK_None);
- if (Type unwrappedTy = underlyingTy->getAnyOptionalObjectType())
- underlyingTy = unwrappedTy;
- if (!underlyingTy->isEqual(importedType))
- importedType = underlyingTy;
- }
- }
-
- if (bridging == Bridgeability::Full) {
+ if (canBridgeTypes(importKind) || importKind == ImportTypeKind::Typedef) {
auto fTy = importedType->castTo<FunctionType>();
FunctionType::ExtInfo einfo = fTy->getExtInfo();
if (einfo.getRepresentation() != FunctionTypeRepresentation::Swift) {
@@ -1356,7 +1341,6 @@
// If we have a bridged Objective-C type and we are allowed to
// bridge, do so.
if (hint == ImportHint::ObjCBridged &&
- bridging == Bridgeability::Full &&
canBridgeTypes(importKind) &&
importKind != ImportTypeKind::PropertyWithReferenceSemantics) {
// id and Any can be bridged without Foundation. There would be
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
index d904d35..d78fe78 100644
--- a/lib/ClangImporter/ImporterImpl.h
+++ b/lib/ClangImporter/ImporterImpl.h
@@ -191,10 +191,13 @@
Full
};
+/// Controls whether a typedef for \p type should name the fully-bridged Swift
+/// type or the original Clang type.
+///
+/// In either case we end up losing sugar at some uses sites, so this is more
+/// about what the right default is.
static inline Bridgeability getTypedefBridgeability(clang::QualType type) {
- return (type->isBlockPointerType() || type->isFunctionType())
- ? Bridgeability::Full
- : Bridgeability::None;
+ return type->isBlockPointerType() ? Bridgeability::Full : Bridgeability::None;
}
/// \brief Describes the kind of the C type that can be mapped to a stdlib
@@ -907,22 +910,45 @@
///
/// \param type The Clang type to import.
///
- /// \param kind The kind of type import we're performing.
+ /// \param kind A classification of the immediate context in which this type
+ /// will be used. Different contexts result in the type being imported
+ /// differently; for example, CF types are normally considered Unmanaged,
+ /// but in parameter position they are known to always be passed at +0.
+ /// See also the \p topLevelBridgeability parameter.
///
/// \param allowNSUIntegerAsInt If true, NSUInteger will be imported as Int
- /// in certain contexts. If false, it will always be imported as UInt.
+ /// in certain contexts. If false, it will always be imported as UInt.
///
- /// \param bridgeability Whether we can bridge types in this context.
- /// This may restrict the ability to bridge types even in contexts
- /// that otherwise allow bridging, such as function results and
- /// parameters.
+ /// \param topLevelBridgeability A classification of the top-level context in
+ /// which this type will be used. This and \p kind are used together to
+ /// determine whether a type can be imported in a more Swifty way than
+ /// a naive translation of its C type. Full bridgeability requires that SIL
+ /// can get back to the original Clang type if it needs to, which implies
+ /// that this type is part of a top-level declaration where we do bridging.
+ /// Without full bridgeability, we can still do some Swifty importing (e.g.
+ /// mapping NSString to String) if we're in an immediate context \p kind
+ /// that allows bridging, but only in cases where Swift's default mapping
+ /// "back" to C is the correct one. If the original type has something
+ /// funny going on, we either have to use a less lossy version of the type
+ /// (ObjCBool rather than Bool) or refuse to import it at all (a block with
+ /// the \c ns_returns_retained attribute).
+ ///
+ /// \param optional If the imported type was a pointer-like type in C, this
+ /// optionality is applied to the resulting Swift type.
+ ///
+ /// \param resugarNSErrorPointer If true, Objective-C's `NSError **` is
+ /// imported as Foundation.NSErrorPointer rather than
+ /// AutoreleasingUnsafeMutablePointer<...>. This is usually desirable
+ /// behavior, but isn't necessary when we use Swift's \c throws anyway.
+ /// Strictly speaking, though, this is a hack used to break cyclic
+ /// dependencies.
///
/// \returns The imported type, or null if this type could
- /// not be represented in Swift.
+ /// not be represented in Swift.
Type importType(clang::QualType type,
ImportTypeKind kind,
bool allowNSUIntegerAsInt,
- Bridgeability bridgeability,
+ Bridgeability topLevelBridgeability,
OptionalTypeKind optional = OTK_ImplicitlyUnwrappedOptional,
bool resugarNSErrorPointer = true);
diff --git a/test/ClangImporter/Inputs/custom-modules/ObjCParseExtras.h b/test/ClangImporter/Inputs/custom-modules/ObjCParseExtras.h
index 27849b3..320e4ab 100644
--- a/test/ClangImporter/Inputs/custom-modules/ObjCParseExtras.h
+++ b/test/ClangImporter/Inputs/custom-modules/ObjCParseExtras.h
@@ -209,3 +209,6 @@
@interface BridgedTypedefs : NSObject
@property (readonly,nonnull) NSArray<NSStringArray> *arrayOfArrayOfStrings;
@end
+
+typedef NSString * _Nonnull (*FPTypedef)(NSString * _Nonnull);
+extern FPTypedef _Nonnull getFP(void);
diff --git a/test/ClangImporter/objc_id_as_any.swift b/test/ClangImporter/objc_id_as_any.swift
index 990435e..4d003b0 100644
--- a/test/ClangImporter/objc_id_as_any.swift
+++ b/test/ClangImporter/objc_id_as_any.swift
@@ -25,7 +25,7 @@
idLover.takesId(x)
idLover.takesId(y)
-install_global_event_handler(idLover) // expected-error {{cannot convert value of type 'NSIdLover' to expected argument type 'event_handler?' (aka 'Optional<@convention(c) (AnyObject) -> ()>')}}
+install_global_event_handler(idLover) // expected-error {{cannot convert value of type 'NSIdLover' to expected argument type 'event_handler?' (aka 'Optional<@convention(c) (Any) -> ()>')}}
// FIXME: this should not type-check!
// Function conversions are not legal when converting to a thin function type.
diff --git a/test/ClangImporter/objc_parse.swift b/test/ClangImporter/objc_parse.swift
index 2f10fe7..b0d587b 100644
--- a/test/ClangImporter/objc_parse.swift
+++ b/test/ClangImporter/objc_parse.swift
@@ -643,3 +643,9 @@
func testBridgedTypedef(bt: BridgedTypedefs) {
let _: Int = bt.arrayOfArrayOfStrings // expected-error{{'[[String]]'}}
}
+
+func testBridgeFunctionPointerTypedefs(fptrTypedef: FPTypedef) {
+ // See also print_clang_bool_bridging.swift.
+ let _: Int = fptrTypedef // expected-error{{'@convention(c) (String) -> String'}}
+ let _: Int = getFP() // expected-error{{'@convention(c) (String) -> String'}}
+}
diff --git a/test/IDE/print_clang_bool_bridging.swift b/test/IDE/print_clang_bool_bridging.swift
index 50b305e..b1d1c94 100644
--- a/test/IDE/print_clang_bool_bridging.swift
+++ b/test/IDE/print_clang_bool_bridging.swift
@@ -42,13 +42,13 @@
func testObjCBoolFnToBlockTypedef(_: @escaping ObjCBoolFn) -> ObjCBoolBlock
func testDarwinBooleanFnToBlockTypedef(_: @escaping DarwinBooleanFn) -> DarwinBooleanBlock
-typealias CBoolFnToBlockType = (CBoolFn) -> CBoolBlock
-typealias ObjCBoolFnToBlockType = (ObjCBoolFn) -> ObjCBoolBlock
-typealias DarwinBooleanFnToBlockType = (DarwinBooleanFn) -> DarwinBooleanBlock
+typealias CBoolFnToBlockType = (CBoolFn) -> (Bool) -> Bool
+typealias ObjCBoolFnToBlockType = (ObjCBoolFn) -> (ObjCBool) -> ObjCBool
+typealias DarwinBooleanFnToBlockType = (DarwinBooleanFn) -> (DarwinBoolean) -> DarwinBoolean
-var globalObjCBoolFnToBlockFP: @convention(c) (ObjCBoolFn) -> @convention(block) (ObjCBool) -> ObjCBool
-var globalObjCBoolFnToBlockFPP: UnsafeMutablePointer<@convention(c) (ObjCBoolFn) -> @convention(block) (ObjCBool) -> ObjCBool>?
-var globalObjCBoolFnToBlockBP: @convention(block) (ObjCBoolFn) -> @convention(block) (ObjCBool) -> ObjCBool
+var globalObjCBoolFnToBlockFP: @convention(c) (ObjCBoolFn) -> (ObjCBool) -> ObjCBool
+var globalObjCBoolFnToBlockFPP: UnsafeMutablePointer<@convention(c) (ObjCBoolFn) -> (ObjCBool) -> ObjCBool>?
+var globalObjCBoolFnToBlockBP: @convention(block) (ObjCBoolFn) -> (ObjCBool) -> ObjCBool
var globalCBoolFn: CBoolFn
var globalObjCBoolFn: ObjCBoolFn
diff --git a/test/IDE/print_omit_needless_words.swift b/test/IDE/print_omit_needless_words.swift
index 7b525d6..9cf56bb 100644
--- a/test/IDE/print_omit_needless_words.swift
+++ b/test/IDE/print_omit_needless_words.swift
@@ -143,7 +143,7 @@
// CHECK-FOUNDATION: func doSomethingElse(with: NSCopying & NSObjectProtocol)
// Note: Function type -> "Function".
-// CHECK-FOUNDATION: func sort(_: @escaping @convention(c) (AnyObject, AnyObject) -> Int)
+// CHECK-FOUNDATION: func sort(_: @escaping @convention(c) (Any, Any) -> Int)
// Note: Plural: NSArray without type arguments -> "Objects".
// CHECK-FOUNDATION: func remove(_: [Any])
diff --git a/test/Interpreter/SDK/Inputs/BlockGlobals/BlockGlobals.h b/test/Interpreter/SDK/Inputs/BlockGlobals/BlockGlobals.h
new file mode 100644
index 0000000..7470413
--- /dev/null
+++ b/test/Interpreter/SDK/Inputs/BlockGlobals/BlockGlobals.h
@@ -0,0 +1,7 @@
+#import <Foundation/Foundation.h>
+
+extern NSString * _Nonnull (^ _Nonnull mutableBlockGlobal)(NSString * _Nonnull);
+extern NSString * _Nonnull (^ _Nonnull const constBlockGlobal)(NSString * _Nonnull);
+
+extern NSString * _Nonnull (* _Nonnull mutableFPGlobal)(NSString * _Nonnull);
+extern NSString * _Nonnull (* _Nonnull const constFPGlobal)(NSString * _Nonnull);
diff --git a/test/Interpreter/SDK/Inputs/BlockGlobals/BlockGlobals.m b/test/Interpreter/SDK/Inputs/BlockGlobals/BlockGlobals.m
new file mode 100644
index 0000000..c3518c3
--- /dev/null
+++ b/test/Interpreter/SDK/Inputs/BlockGlobals/BlockGlobals.m
@@ -0,0 +1,20 @@
+#import "BlockGlobals.h"
+
+NSString *(^mutableBlockGlobal)(NSString *) = ^ NSString *(NSString *arg) {
+ return [@"default mutable block: " stringByAppendingString: arg];
+};
+NSString *(^ const constBlockGlobal)(NSString *) = ^ NSString *(NSString *arg) {
+ return [@"default const block: " stringByAppendingString: arg];
+};
+
+static NSString *appendToDefault(NSString *arg) {
+ return [@"default mutable FP: " stringByAppendingString: arg];
+}
+
+NSString *(*mutableFPGlobal)(NSString *) = &appendToDefault;
+
+
+static NSString *appendToDefaultConst(NSString *arg) {
+ return [@"default const FP: " stringByAppendingString: arg];
+}
+NSString *(* const constFPGlobal)(NSString *) = &appendToDefaultConst;
diff --git a/test/Interpreter/SDK/block_globals.swift b/test/Interpreter/SDK/block_globals.swift
new file mode 100644
index 0000000..d22559c
--- /dev/null
+++ b/test/Interpreter/SDK/block_globals.swift
@@ -0,0 +1,29 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-clang -fobjc-arc %S/Inputs/BlockGlobals/BlockGlobals.m -c -o %t/BlockGlobals.o
+// RUN: %target-build-swift %s -import-objc-header %S/Inputs/BlockGlobals/BlockGlobals.h %t/BlockGlobals.o -o %t/main
+// RUN: %target-run %t/main
+
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+import Foundation
+import StdlibUnittest
+
+var BlockGlobalsTestSuite = TestSuite("BlockGlobals")
+
+BlockGlobalsTestSuite.test("const block") {
+ expectEqual((@convention(block) (String) -> String).self as Any.Type, type(of: constBlockGlobal))
+ expectEqual("default const block: abc", constBlockGlobal("abc"))
+}
+
+BlockGlobalsTestSuite.test("const function pointer") {
+ expectEqual((@convention(c) (String) -> String).self as Any.Type, type(of: constFPGlobal))
+ expectEqual("default const FP: abc", constFPGlobal("abc"))
+}
+
+// FIXME: Add tests for mutable globals, including mutating them, once the
+// compiler supports it, as well as loading from the const globals without
+// immediately calling them.
+
+runAllTests()
