Skip to content

Commit 2cbd70e

Browse files
committed
Make SwiftValue hash/equality work the same as SwiftObject
Update PR #68720 with lessons learned in reviewing #69464 Background: * SwiftValue can expose Swift value types (structs/enums) to ObjC by wrapping them in an Obj-C object on the heap * SwiftObject is the Obj-C type that Swift class objects all inherit from (when viewed from Obj-C). This allows arbitrary Swift class objects to be passed into Obj-C. History: * PR #4124 made Obj-C `-hash` and `-isEqual:` work for SwiftValue for Hashable Swift types * PR #68720 extended SwiftValue to also support Equatable Swift types * PR #69464 added similar support to SwiftObject In the process of working through #69464, we found a better way to handle an ObjC request for `-hash` for a type that is Swift Equatable but not Hashable. This PR updates SwiftValue to use the same approach. The approach considers three cases: 1. A Hashable type can forward both `-hash` and `-isEqual:` to the Swift object. 2. A type that is neither Equatable nor Hashable can implement `-isEqual:` as the identity check and `-hash` as returning the address of the object in memory. 3. A type is that Equatable but not Hashable is more complex. In this last case, we can easily forward `-isEqual:` to the Equatable conformance but ObjC also requires us to always provide a compatible `-hash` implementation. The only way to do this is to have `-hash` return a constant, but that is a very bad idea in general, so we're also including a log message whenever we see a request for `-hash` on a Swift value that is Equatable but not Hashable. To limit performance problems from the logging itself, we emit the log message only once for each type.
1 parent b7d8a9b commit 2cbd70e

File tree

3 files changed

+563
-6
lines changed

3 files changed

+563
-6
lines changed

stdlib/public/runtime/SwiftValue.mm

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929
#include "swift/Runtime/Metadata.h"
3030
#include "swift/Runtime/ObjCBridge.h"
3131
#include "swift/Runtime/Debug.h"
32+
#include "swift/Threading/Mutex.h"
3233
#include "Private.h"
3334
#include "SwiftEquatableSupport.h"
3435
#include "SwiftHashableSupport.h"
3536
#include <objc/runtime.h>
3637
#include <Foundation/Foundation.h>
3738

3839
#include <new>
40+
#include <unordered_set>
3941

4042
using namespace swift;
4143
using namespace swift::hashable_support;
@@ -446,15 +448,43 @@ - (BOOL)isEqual:(id)other {
446448
}
447449

448450
- (NSUInteger)hash {
451+
// If Swift type is Hashable, get the hash value from there
449452
auto selfHeader = getSwiftValueHeader(self);
450453
auto hashableConformance = selfHeader->getHashableConformance();
451-
if (!hashableConformance) {
452-
return (NSUInteger)self;
454+
if (hashableConformance) {
455+
return _swift_stdlib_Hashable_hashValue_indirect(
456+
getSwiftValuePayload(self,
457+
getSwiftValuePayloadAlignMask(selfHeader->type)),
458+
selfHeader->type, hashableConformance);
453459
}
454-
return _swift_stdlib_Hashable_hashValue_indirect(
455-
getSwiftValuePayload(self,
456-
getSwiftValuePayloadAlignMask(selfHeader->type)),
457-
selfHeader->type, hashableConformance);
460+
461+
// If Swift type is Equatable but not Hashable,
462+
// we have to return something here that is compatible
463+
// with the `isEqual:` above.
464+
auto equatableConformance = selfHeader->getEquatableConformance();
465+
if (equatableConformance) {
466+
// Warn once per type about this
467+
auto metadata = getSwiftValueTypeMetadata(self);
468+
static Lazy<std::unordered_set<const Metadata *>> warned;
469+
static LazyMutex warnedLock;
470+
LazyMutex::ScopedLock guard(warnedLock);
471+
auto result = warned.get().insert(metadata);
472+
auto inserted = std::get<1>(result);
473+
if (inserted) {
474+
TypeNamePair typeName = swift_getTypeName(metadata, true);
475+
warning(0,
476+
"Obj-C `-hash` invoked on a Swift value of type `%s` that is Equatable but not Hashable; "
477+
"this can lead to severe performance problems.\n",
478+
typeName.data);
479+
}
480+
// Constant value (yuck!) is the only choice here
481+
return (NSUInteger)1;
482+
}
483+
484+
// If the Swift type is neither Equatable nor Hashable,
485+
// then we can hash the identity, which should be pretty
486+
// good in practice.
487+
return (NSUInteger)self;
458488
}
459489

460490
static id getValueDescription(__SwiftValue *self) {

0 commit comments

Comments
 (0)