Skip to content

Commit b6f9677

Browse files
authored
Merge pull request #35650 from tbkka/tbkka/compatibleOptionalAnyHashableCasting
2 parents 19a4df3 + 086dc14 commit b6f9677

File tree

2 files changed

+80
-14
lines changed

2 files changed

+80
-14
lines changed

stdlib/public/runtime/DynamicCast.cpp

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -731,26 +731,49 @@ tryCastToAnyHashable(
731731
// TODO: Implement a fast path for NSString->AnyHashable casts.
732732
// These are incredibly common because an NSDictionary with
733733
// NSString keys is bridged by default to [AnyHashable:Any].
734-
// Until this is implemented, fall through to the default case
735-
SWIFT_FALLTHROUGH;
734+
// Until this is implemented, fall through to the general case
735+
break;
736736
#else
737-
// If no Obj-C interop, just fall through to the default case.
738-
SWIFT_FALLTHROUGH;
737+
// If no Obj-C interop, just fall through to the general case.
738+
break;
739739
#endif
740740
}
741-
default: {
742-
auto hashableConformance = reinterpret_cast<const HashableWitnessTable *>(
743-
swift_conformsToProtocol(srcType, &HashableProtocolDescriptor));
744-
if (hashableConformance) {
745-
_swift_convertToAnyHashableIndirect(srcValue, destLocation,
746-
srcType, hashableConformance);
747-
return DynamicCastResult::SuccessViaCopy;
748-
} else {
749-
return DynamicCastResult::Failure;
741+
case MetadataKind::Optional: {
742+
// Until SR-9047 fixes the interactions between AnyHashable and Optional, we
743+
// avoid directly injecting Optionals. In particular, this allows
744+
// casts from [String?:String] to [AnyHashable:Any] to work the way people
745+
// expect. Otherwise, without SR-9047, the resulting dictionary can only be
746+
// indexed with an explicit Optional<String>, not a plain String.
747+
// After SR-9047, we can consider dropping this special case entirely.
748+
749+
// !!!! This breaks compatibility with compiler-optimized casts
750+
// (which just inject) and violates the Casting Spec. It just preserves
751+
// the behavior of the older casting code until we can clean things up.
752+
auto srcInnerType = cast<EnumMetadata>(srcType)->getGenericArgs()[0];
753+
unsigned sourceEnumCase = srcInnerType->vw_getEnumTagSinglePayload(
754+
srcValue, /*emptyCases=*/1);
755+
auto nonNil = (sourceEnumCase == 0);
756+
if (nonNil) {
757+
return DynamicCastResult::Failure; // Our caller will unwrap the optional and try again
750758
}
759+
// Else Optional is nil -- the general case below will inject it
760+
break;
751761
}
762+
default:
763+
break;
764+
}
765+
766+
767+
// General case: If it conforms to Hashable, we cast it
768+
auto hashableConformance = reinterpret_cast<const HashableWitnessTable *>(
769+
swift_conformsToProtocol(srcType, &HashableProtocolDescriptor));
770+
if (hashableConformance) {
771+
_swift_convertToAnyHashableIndirect(srcValue, destLocation,
772+
srcType, hashableConformance);
773+
return DynamicCastResult::SuccessViaCopy;
774+
} else {
775+
return DynamicCastResult::Failure;
752776
}
753-
return DynamicCastResult::Failure;
754777
}
755778

756779
static DynamicCastResult

test/Casting/Casts.swift

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,4 +881,47 @@ CastsTests.test("NSDictionary -> Dictionary casting [SR-12025]") {
881881
}
882882
#endif
883883

884+
// Casting optionals to AnyHashable is a little peculiar
885+
// TODO: It would be nice if AnyHashable(Optional("Foo")) == AnyHashable("Foo")
886+
// (including as dictionary keys). That would make this a lot less confusing.
887+
CastsTests.test("Optional cast to AnyHashable") {
888+
let d: [String?: String] = ["FooKey": "FooValue", nil: "NilValue"]
889+
// In Swift 5.3, this cast DOES unwrap the non-nil key
890+
// We've deliberately tried to preserve that behavior in Swift 5.4
891+
let d2 = d as [AnyHashable: String]
892+
893+
// After SR-9047, all four of the following should work:
894+
let d3 = d2["FooKey" as String? as AnyHashable]
895+
expectNil(d3)
896+
let d4 = d2["FooKey" as String?]
897+
expectNil(d4)
898+
let d5 = d2["FooKey"]
899+
expectNotNil(d5)
900+
let d6 = d2["FooKey" as AnyHashable]
901+
expectNotNil(d6)
902+
903+
// The nil key should be preserved and still function
904+
let d7 = d2[String?.none as AnyHashable]
905+
expectNotNil(d7)
906+
907+
// Direct casts via the runtime unwrap the optional
908+
let a: String = "Foo"
909+
let ah: AnyHashable = a
910+
let b: String? = a
911+
let bh = runtimeCast(b, to: AnyHashable.self)
912+
expectEqual(bh, ah)
913+
914+
// Direct casts that don't go through the runtime don't unwrap the optional
915+
// This is inconsistent with the runtime cast behavior above. We should
916+
// probably change the runtime behavior above to work the same as this,
917+
// but that should wait until SR-9047 lands.
918+
let x: String = "Baz"
919+
let xh = x as AnyHashable
920+
let y: String? = x
921+
let yh = y as AnyHashable // Doesn't unwrap the optional
922+
// xh is AnyHashable("Baz")
923+
// yh is AnyHashable(Optional("Baz"))
924+
expectNotEqual(xh, yh)
925+
}
926+
884927
runAllTests()

0 commit comments

Comments
 (0)