Skip to content

Turn on ‘as’ bridging on Linux. #16022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 17, 2018
Merged

Turn on ‘as’ bridging on Linux. #16022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b945593
Turn on ‘as’ bridging on Darwin.
millenomi Mar 27, 2018
7b8db8f
Remove last vestige of printf debugging.
millenomi May 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions lib/SIL/DynamicCasts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1134,22 +1134,19 @@ bool swift::canUseScalarCheckedCastInstructions(SILModule &M,
if (!objectType.isAnyClassReferenceType())
return false;

if (M.getASTContext().LangOpts.EnableObjCInterop) {
auto super = archetype->getSuperclass();
if (super.isNull())
return false;

// A base class constraint that isn't NSError rules out the archetype being
// bound to NSError.
if (auto nserror = M.Types.getNSErrorType())
return !super->isEqual(nserror);
// If NSError wasn't loaded, any base class constraint must not be NSError.
return true;
} else {
// If ObjC bridging isn't enabled, we can do a scalar cast from any
// reference type to any class-constrained archetype.
return archetype->requiresClass();
// A base class constraint that isn't NSError rules out the archetype being
// bound to NSError.
if (M.getASTContext().LangOpts.EnableObjCInterop) {
if (auto nserror = M.Types.getNSErrorType())
return !super->isEqual(nserror);
}

// If NSError wasn't loaded, any base class constraint must not be NSError.
return true;
}

if (M.getASTContext().LangOpts.EnableObjCInterop
Expand Down
5 changes: 1 addition & 4 deletions lib/Sema/CSApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3686,10 +3686,7 @@ namespace {
auto *locator = cs.getConstraintLocator(expr);

if (!choice) {
if (tc.Context.LangOpts.EnableObjCInterop)
choice = solution.getDisjunctionChoice(locator);
else
choice = 0;
choice = solution.getDisjunctionChoice(locator);
}

// Handle the coercion/bridging of the underlying subexpression, where
Expand Down
22 changes: 7 additions & 15 deletions lib/Sema/CSSimplify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3627,11 +3627,6 @@ ConstraintSystem::simplifyBridgingConstraint(Type type1,
Type type2,
TypeMatchOptions flags,
ConstraintLocatorBuilder locator) {
// There's no bridging without ObjC interop, so we shouldn't have set up
// bridging constraints without it.
assert(TC.Context.LangOpts.EnableObjCInterop
&& "bridging constraint w/o ObjC interop?!");

TypeMatchOptions subflags = getDefaultDecompositionOptions(flags);

/// Form an unresolved result.
Expand Down Expand Up @@ -5076,14 +5071,11 @@ void ConstraintSystem::addExplicitConversionConstraint(
coerceConstraint->setFavored();
constraints.push_back(coerceConstraint);

// Bridging.
if (getASTContext().LangOpts.EnableObjCInterop) {
// The source type can be explicitly converted to the destination type.
Constraint *bridgingConstraint =
Constraint::create(*this, ConstraintKind::BridgingConversion,
fromType, toType, locatorPtr);
constraints.push_back(bridgingConstraint);
}
// The source type can be explicitly converted to the destination type.
Constraint *bridgingConstraint =
Constraint::create(*this, ConstraintKind::BridgingConversion,
fromType, toType, locatorPtr);
constraints.push_back(bridgingConstraint);

if (allowFixes && shouldAttemptFixes()) {
Constraint *downcastConstraint =
Expand All @@ -5094,8 +5086,8 @@ void ConstraintSystem::addExplicitConversionConstraint(
}

addDisjunctionConstraint(constraints, locator,
getASTContext().LangOpts.EnableObjCInterop && allowFixes ? RememberChoice
: ForgetChoice);
allowFixes ? RememberChoice
: ForgetChoice);
}

ConstraintSystem::SolutionKind
Expand Down
4 changes: 0 additions & 4 deletions lib/Sema/Constraint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,10 +612,6 @@ Constraint *Constraint::create(ConstraintSystem &cs, ConstraintKind kind,
assert((kind != ConstraintKind::LiteralConformsTo) ||
second->is<ProtocolType>());

// Bridging constraints require bridging to be enabled.
assert(kind != ConstraintKind::BridgingConversion
|| cs.TC.Context.LangOpts.EnableObjCInterop);

// Create the constraint.
unsigned size = totalSizeToAlloc<TypeVariableType*>(typeVars.size());
void *mem = cs.getAllocator().Allocate(size, alignof(Constraint));
Expand Down
8 changes: 3 additions & 5 deletions lib/Sema/TypeCheckConstraints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2881,8 +2881,7 @@ bool TypeChecker::isExplicitlyConvertibleTo(Type type1, Type type2,

bool TypeChecker::isObjCBridgedTo(Type type1, Type type2, DeclContext *dc,
bool *unwrappedIUO) {
return (Context.LangOpts.EnableObjCInterop &&
typesSatisfyConstraint(type1, type2,
return (typesSatisfyConstraint(type1, type2,
/*openArchetypes=*/false,
ConstraintKind::BridgingConversion,
dc, unwrappedIUO));
Expand Down Expand Up @@ -3394,9 +3393,8 @@ CheckedCastKind TypeChecker::typeCheckCheckedCast(Type fromType,
}

// Check for a bridging conversion.
// Anything bridges to AnyObject in ObjC interop mode.
if (Context.LangOpts.EnableObjCInterop
&& toType->isAnyObject())
// Anything bridges to AnyObject.
if (toType->isAnyObject())
return CheckedCastKind::BridgingCoercion;

// Do this check later in Swift 3 mode so that we check for NSNumber and
Expand Down
4 changes: 4 additions & 0 deletions stdlib/public/SDK/ObjectiveC/ObjectiveC.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//

#if _runtime(_ObjC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we try to build the ObjectiveC module on non-ObjC-interop platforms? I didn't see a CMakeLists.txt change to that effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can revert this if it’s unneeded. Part of the debugging cycle for this patch was building it on Darwin simulating the Linux situation so that I could iterate on it with Xcode; it may be a leftover from that.


@_exported
import ObjectiveC
import _SwiftObjectiveCOverlayShims
Expand Down Expand Up @@ -225,3 +227,5 @@ extension NSObject : CVarArg {
}
}

#endif

200 changes: 199 additions & 1 deletion stdlib/public/core/BridgeObjectiveC.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
//
//===----------------------------------------------------------------------===//

#if _runtime(_ObjC)
/// A Swift Array or Dictionary of types conforming to
/// `_ObjectiveCBridgeable` can be passed to Objective-C as an NSArray or
/// NSDictionary, respectively. The elements of the resulting NSArray
Expand Down Expand Up @@ -83,6 +82,8 @@ public protocol _ObjectiveCBridgeable {
-> Self
}

#if _runtime(_ObjC)

//===--- Bridging for metatypes -------------------------------------------===//

/// A stand-in for a value of metatype type.
Expand Down Expand Up @@ -640,3 +641,200 @@ public func _getObjCTypeEncoding<T>(_ type: T.Type) -> UnsafePointer<Int8> {
}

#endif

//===--- Bridging without the ObjC runtime --------------------------------===//

#if !_runtime(_ObjC)

/// Convert `x` from its Objective-C representation to its Swift
/// representation.
/// COMPILER_INTRINSIC
@_inlineable // FIXME(sil-serialize-all)
public func _forceBridgeFromObjectiveC_bridgeable<T:_ObjectiveCBridgeable> (
_ x: T._ObjectiveCType,
_: T.Type
) -> T {
var result: T?
T._forceBridgeFromObjectiveC(x, result: &result)
return result!
}

/// Attempt to convert `x` from its Objective-C representation to its Swift
/// representation.
/// COMPILER_INTRINSIC
@_inlineable // FIXME(sil-serialize-all)
public func _conditionallyBridgeFromObjectiveC_bridgeable<T:_ObjectiveCBridgeable>(
_ x: T._ObjectiveCType,
_: T.Type
) -> T? {
var result: T?
T._conditionallyBridgeFromObjectiveC (x, result: &result)
return result
}

public // SPI(Foundation)
protocol _NSSwiftValue: class {
init(_ value: Any)
var value: Any { get }
static var null: AnyObject { get }
}

@usableFromInline
internal class _SwiftValue {
@usableFromInline
let value: Any
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could avoid double-indirecting using a small class hierarchy:

open class _SwiftValue {}

internal class _SwiftValueOf<T>: _SwiftValue {
  let value: T
  init(_ value: T) { self.value = value }
}

@_silgen_name("_swift_makeSwiftValue")
func _makeSwiftValue<T>(_ value: T) -> _SwiftValue { return _SwiftValueOf(value) }

This might also let you represent null as a singleton subclass instead of as a wrapper around a specific kind of Optional. Maybe you could also make it so that Foundation's preexisting _SwiftValue and NSNull are Swift._SwiftValue subclasses, so that they're substitutable for stdlib _SwiftValues, instead of the _NSSwiftValue/_SwiftValueFlavor hacks too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot reparent NSNull, and I would be loathe to reparent _SwiftValue for the reasons outlined below. Unless we reparent NSObject, which is possible but would be really weird.


@usableFromInline
init(_ value: Any) {
self.value = value
}

@usableFromInline
static let null = _SwiftValue(Optional<Any>.none as Any)
}

/// COMPILER_INTRISIC
@_silgen_name("swift_unboxFromSwiftValueWithType")
public func swift_unboxFromSwiftValueWithType<T>(
_ source: inout AnyObject,
_ result: UnsafeMutablePointer<T>
) -> Bool {

if source === _nullPlaceholder {
if let unpacked = Optional<Any>.none as? T {
result.initialize(to: unpacked)
return true
}
}

if let box = source as? _SwiftValue {
if let value = box.value as? T {
result.initialize(to: value)
return true
}
} else if let box = source as? _NSSwiftValue {
if let value = box.value as? T {
result.initialize(to: value)
return true
}
}

return false
}

@_silgen_name("_swift_extractDynamicValue")
public func _extractDynamicValue<T>(_ value: T) -> AnyObject?

@_silgen_name("_swift_bridgeToObjectiveCUsingProtocolIfPossible")
public func _bridgeToObjectiveCUsingProtocolIfPossible<T>(_ value: T) -> AnyObject?

@usableFromInline
protocol _Unwrappable {
func unwrap() -> Any?
}

extension Optional: _Unwrappable {
func unwrap() -> Any? {
return self
}
}

// This is a best-effort tripmine for detecting the situation
// (which should never happen) of Swift._SwiftValue and
// Foundation._SwiftValue/Foundation.NSNull being used
// in the same process.

@usableFromInline
internal enum _SwiftValueFlavor: Equatable {
case stdlib
case foundation
}

@usableFromInline
func _currentSwiftValueFlavor() -> _SwiftValueFlavor {
if _typeByName("Foundation._SwiftValue") as? _NSSwiftValue.Type != nil {
return .foundation
} else {
return .stdlib
}
}

@usableFromInline
internal var _selectedSwiftValueFlavor: _SwiftValueFlavor = _currentSwiftValueFlavor()

@usableFromInline
internal func _assertSwiftValueFlavorIsConsistent() {
assert(_selectedSwiftValueFlavor == _currentSwiftValueFlavor())
}

@usableFromInline
internal var _nullPlaceholder: AnyObject {
_assertSwiftValueFlavorIsConsistent()
if let foundationType = _typeByName("Foundation._SwiftValue") as? _NSSwiftValue.Type {
return foundationType.null
} else {
return _SwiftValue.null
}
}

@usableFromInline
func _makeSwiftValue(_ value: Any) -> AnyObject {
_assertSwiftValueFlavorIsConsistent()
if let foundationType = _typeByName("Foundation._SwiftValue") as? _NSSwiftValue.Type {
return foundationType.init(value)
} else {
return _SwiftValue(value)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a plan to retire Foundation._SwiftValue once this is all done?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that _SwiftValue is an NSObject subclass on Apple platforms, and we probably can't undo that.

Copy link
Contributor

@jckarter jckarter May 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does that become important in practice? We don't promise that on Darwin even, we "just haven't gotten around" to making it an independent root class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It becomes important when it interacts with CF API. Right now, calling CFCopy() will go through the NSCopying conformance that all SwiftValues implicitly have to, and that is still possible on Linux as well (you can toll-free bridge swift-corelibs-foundation’s NSArray/Dictionary to CFArray/DictionaryRef the same way you can on Darwin, and these may contain _SwiftValues and be set up to copy.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It is also secondarily important for two reasons:

  • It allows Linux to respond to is/as? NSObject/NSCopying the same way Darwin does, which can be a subtle issue in codebases where that question is posed in code that doesn’t know it may be receiving a boxed value;
  • Existing boxing code in s-c-f already assumes that boxed values can be freely casted to NSObject (and uses its Hashable conformance to eg use them as dictionary keys); the churn was pretty big and would have introduced a number of forced casts and uses of AnyHashable, so I went instead with the solution that also he the property of being the closest match to Darwin.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a high level, I don't think changing the boxing behavior dynamically based on the presence of Foundation is workable—what happens when a program that doesn't use Foundation dynamically loads a library that does? as? NSObject working is not something we want to guarantee works with plain boxed values on Darwin. We control the CF implementation corelibs uses on Linux, so could we make CFCopy do the right thing with Swift boxes?

If we really must make boxed values be NSObject subclasses when Foundation is loaded, we should look into doing so in a way that doesn't invalidate boxed values that exist before Foundation is loaded. For instance, we could make it so that boxing uses heap objects that start out with non-class heap metadata, and overwrite the metadata with an NSObject subclass when Foundation gets loaded.


/// Bridge an arbitrary value to an Objective-C object.
///
/// - If `T` is a class type, it is always bridged verbatim, the function
/// returns `x`;
///
/// - otherwise, if `T` conforms to `_ObjectiveCBridgeable`,
/// returns the result of `x._bridgeToObjectiveC()`;
///
/// - otherwise, we use **boxing** to bring the value into Objective-C.
/// The value is wrapped in an instance of a private Objective-C class
/// that is `id`-compatible and dynamically castable back to the type of
/// the boxed value, but is otherwise opaque.
///
/// COMPILER_INTRINSIC
@inlinable // FIXME(sil-serialize-all)
public func _bridgeAnythingToObjectiveC<T>(_ x: T) -> AnyObject {
var done = false
var result: AnyObject!

var source: Any = x

if let dynamicSource = _extractDynamicValue(x) {
result = dynamicSource as AnyObject
done = true
}

if !done, let wrapper = source as? _Unwrappable {
if let value = wrapper.unwrap() {
result = value as AnyObject
} else {
result = _nullPlaceholder
}

done = true
}

if !done {
if type(of: source) as? AnyClass != nil {
result = unsafeBitCast(x, to: AnyObject.self)
} else if let object = _bridgeToObjectiveCUsingProtocolIfPossible(source) {
result = object
} else {
result = _makeSwiftValue(source)
}
}

return result
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this would be best written in C++ to have full access to the low-level metadata APIs. Keeping the entire implementation would be cleaner, and probably lead to more understandable code, than doing the oblique things you need to approximate this in Swift slowly and opaquely, such as sprinkling new protocols like _Unwrappable for peeling optionals or new small bits of runtime/stdlib SPI like _extractDynamicValue for peeling existentials. With Optional we also need to be careful that boxing preserves the difference between different levels of .none nesting in nested Optionals—for T??, .none and .some(.none) should be able to round-trip through a box as distinct values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of why I went with Swift over C++ was the issue of correctness of memory management; is there documentation on the expectations and SPI? My understanding is that with the current work the documentation in the repository may no longer apply.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main thing that would've changed since then is that Swift functions now take arguments +0 by default (and we have macros to try to handle both +1 and +0 conventions as a transitional/fallback mechanism, though I'm not sure whether that's important for new code, cc @gottesmm). The Darwin implementation of bridging lives largely in the C++ runtime, and we have fairly extensive runtime tests that you should be able to gradually enable that check for object lifetime issues.


#endif // !_runtime(_ObjC)

4 changes: 2 additions & 2 deletions stdlib/public/core/Codable.swift.gyb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,7 @@ public enum EncodingError : Error {
public var _userInfo: AnyObject? {
// The error dictionary must be returned as an AnyObject. We can do this
// only on platforms with bridging, unfortunately.
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
#if _runtime(_ObjC)
let context: Context
switch self {
case .invalidValue(_, let c): context = c
Expand Down Expand Up @@ -1281,7 +1281,7 @@ public enum DecodingError : Error {
public var _userInfo: AnyObject? {
// The error dictionary must be returned as an AnyObject. We can do this
// only on platforms with bridging, unfortunately.
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
#if _runtime(_ObjC)
let context: Context
switch self {
case .keyNotFound(_, let c): context = c
Expand Down
Loading