Skip to content

NSNumber already preserves whether a value was originally boolean. #4366

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.

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 1 commit into from
Aug 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 2 additions & 12 deletions stdlib/public/SDK/Foundation/Foundation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,6 @@ internal func _swift_Foundation_TypePreservingNSNumberWithCGFloat(
_ value: CGFloat
) -> NSNumber

@_silgen_name("_swift_Foundation_TypePreservingNSNumberWithBool")
internal func _swift_Foundation_TypePreservingNSNumberWithBool(
_ value: Bool
) -> NSNumber

@_silgen_name("_swift_Foundation_TypePreservingNSNumberGetKind")
internal func _swift_Foundation_TypePreservingNSNumberGetKind(
_ value: NSNumber
Expand Down Expand Up @@ -204,11 +199,6 @@ internal func _swift_Foundation_TypePreservingNSNumberGetAsCGFloat(
_ value: NSNumber
) -> CGFloat

@_silgen_name("_swift_Foundation_TypePreservingNSNumberGetAsBool")
internal func _swift_Foundation_TypePreservingNSNumberGetAsBool(
_ value: NSNumber
) -> Bool

// Conversions between NSNumber and various numeric types. The
// conversion to NSNumber is automatic (auto-boxing), while conversion
// back to a specific numeric type requires a cast.
Expand Down Expand Up @@ -348,7 +338,7 @@ extension Bool: _ObjectiveCBridgeable {

@_semantics("convertToObjectiveC")
public func _bridgeToObjectiveC() -> NSNumber {
return _swift_Foundation_TypePreservingNSNumberWithBool(self)
return NSNumber(value: self)
}

public static func _forceBridgeFromObjectiveC(
Expand Down Expand Up @@ -456,7 +446,7 @@ extension NSNumber : _HasCustomAnyHashableRepresentation {
case .CoreGraphicsCGFloat:
return AnyHashable(_swift_Foundation_TypePreservingNSNumberGetAsCGFloat(self))
case .SwiftBool:
return AnyHashable(_swift_Foundation_TypePreservingNSNumberGetAsBool(self))
return AnyHashable(self.boolValue)
}
}
}
Expand Down
21 changes: 12 additions & 9 deletions stdlib/public/SDK/Foundation/TypePreservingNSNumber.mm
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,13 @@ - (const char *)objCType {
case SwiftCGFloat:
return @encode(CGFloat);
case SwiftBool:
return @encode(bool);
// Bool is represented by CFBoolean.
break;
}
swift::swift_reportError(
/* flags = */ 0,
"_SwiftTypePreservingNSNumber.tag is corrupted.\n");
abort();
}

- (void)getValue:(void *)value {
Expand All @@ -102,12 +104,13 @@ - (void)getValue:(void *)value {
memcpy(value, self->storage, sizeof(CGFloat));
return;
case SwiftBool:
memcpy(value, self->storage, sizeof(bool));
return;
// Bool is represented by CFBoolean.
break;
}
swift::swift_reportError(
/* flags = */ 0,
"_SwiftTypePreservingNSNumber.tag is corrupted.\n");
abort();
}

#define DEFINE_ACCESSOR(C_TYPE, METHOD_NAME) \
Expand Down Expand Up @@ -139,14 +142,14 @@ - (C_TYPE)METHOD_NAME { \
return result; \
} \
case SwiftBool: { \
bool result; \
memcpy(&result, self->storage, sizeof(result)); \
return result; \
/* Bool is represented by CFBoolean. */ \
break; \
} \
} \
swift::swift_reportError( \
/* flags = */ 0, \
"_SwiftTypePreservingNSNumber.tag is corrupted.\n"); \
abort(); \
}

DEFINE_ACCESSOR(char, charValue)
Expand Down Expand Up @@ -178,16 +181,17 @@ - (Class)classForCoder {
DEFINE_INIT(float, Float)
DEFINE_INIT(double, Double)
DEFINE_INIT(CGFloat, CGFloat)
DEFINE_INIT(bool, Bool)

#undef DEFINE_INIT

SWIFT_CC(swift) extern "C" uint32_t
_swift_Foundation_TypePreservingNSNumberGetKind(
NSNumber *NS_RELEASES_ARGUMENT _Nonnull self_) {
uint32_t result = 0;
if ([self_ isKindOfClass: [_SwiftTypePreservingNSNumber class]]) {
if ([self_ isKindOfClass:[_SwiftTypePreservingNSNumber class]]) {
result = ((_SwiftTypePreservingNSNumber *) self_)->tag;
} else if (CFGetTypeID(self_) == CFBooleanGetTypeID()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this probably should be a strcmp of self.objCType and @encode(BOOL)

Copy link
Contributor

Choose a reason for hiding this comment

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

assert(strcmp(@YES.objCType, @encode(BOOL)) == 0);

where as CFGetTypeID of a subclass of NSNumber may not return CFBooleanGetTypeID() unless _cfTypeID is overridden.

result = SwiftBool;
}
[self_ release];
return result;
Expand All @@ -212,7 +216,6 @@ - (Class)classForCoder {
DEFINE_GETTER(float, Float)
DEFINE_GETTER(double, Double)
DEFINE_GETTER(CGFloat, CGFloat)
DEFINE_GETTER(bool, Bool)

#undef DEFINE_GETTER

Expand Down
17 changes: 15 additions & 2 deletions validation-test/stdlib/NSNumberBridging.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ NSNumberTests.test("_SwiftTypePreservingNSNumber.init(coder:)")
NSNumberTests.test("_SwiftTypePreservingNSNumber.copy(zone:)") {
let n: NSNumber = (42 as Int)._bridgeToObjectiveC()
expectTrue(isTypePreservingNSNumber(n))
let copy = n.copy() as! AnyObject
let copy = n.copy() as AnyObject
expectTrue(n === copy)
}

Expand Down Expand Up @@ -197,7 +197,11 @@ NSNumberTests.test("_SwiftTypePreservingNSNumber(${Self}).getValue(_:), objCType
.forEach(in: ${Self}._interestingValues) {
input in
let bridgedNSNumber = input._bridgeToObjectiveC()
% if Self == 'Bool':
expectTrue(CFGetTypeID(bridgedNSNumber) == CFBooleanGetTypeID())
% else:
expectTrue(isTypePreservingNSNumber(bridgedNSNumber))
% end

let expectedObjCType: String
% if Self == 'Int':
Expand Down Expand Up @@ -229,7 +233,9 @@ NSNumberTests.test("_SwiftTypePreservingNSNumber(${Self}).getValue(_:), objCType
_UnknownArchError()
#endif
% elif Self == 'Bool':
expectedObjCType = "B"
// NSNumber always encodes booleans as 'signed char', even on platforms where
// ObjCBool is a true Bool. This is a very old compatibility concern.
expectedObjCType = "c"
% else:
_UnknownTypeError()
% end
Expand All @@ -247,13 +253,20 @@ NSNumberTests.test("${Self} bridges to NSNumber (actually _SwiftTypePreservingNS
input in
// Bridged NSNumbers preserve the Swift type when put into AnyHashable.
let bridgedNSNumber = input._bridgeToObjectiveC()
% if Self == 'Bool':
expectTrue(CFGetTypeID(bridgedNSNumber) == CFBooleanGetTypeID())
% else:
expectTrue(isTypePreservingNSNumber(bridgedNSNumber))
% end
expectNotEmpty(bridgedNSNumber._toCustomAnyHashable())

// Explicitly constructed NSNumbers don't have a special AnyHashable
// representation.
% if Self == 'CGFloat':
let explicitNSNumber = NSNumber(value: input.native)
% elif Self == 'Bool':
// Bool actually /is/ type-preserving for NSNumber. Use a dummy value instead.
let explicitNSNumber = NSNumber(value: (input ? 1 : 0) as Int8)
% else:
let explicitNSNumber = NSNumber(value: input)
% end
Expand Down