Skip to content

Commit

Permalink
Merge pull request #9429 from jckarter/dont-resort-protocols-at-runtime
Browse files Browse the repository at this point in the history
Runtime: swift_getExistentialTypeMetadata should trust the compiler's ordering of protocols in compositions.
jckarter authored May 10, 2017
2 parents 8723187 + 439d5d6 commit 36127b2
Showing 3 changed files with 9 additions and 49 deletions.
12 changes: 4 additions & 8 deletions stdlib/public/runtime/Demangle.cpp
Original file line number Diff line number Diff line change
@@ -161,14 +161,10 @@ swift::_swift_buildDemanglingForMetadata(const Metadata *type,
auto proto_list = Dem.createNode(Node::Kind::ProtocolList);
proto_list->addChild(type_list, Dem);

// Sort the protocols by their mangled names.
// The ordering in the existential type metadata is by metadata pointer,
// which isn't necessarily stable across invocations.
std::sort(protocols.begin(), protocols.end(),
[](const ProtocolDescriptor *a, const ProtocolDescriptor *b) -> bool {
return strcmp(a->Name, b->Name) < 0;
});

// The protocol descriptors should be pre-sorted since the compiler will
// only ever make a swift_getExistentialTypeMetadata invocation using
// its canonical ordering of protocols.

for (auto *protocol : protocols) {
// The protocol name is mangled as a type symbol, with the _Tt prefix.
StringRef ProtoName(protocol->Name);
5 changes: 3 additions & 2 deletions stdlib/public/runtime/Metadata.cpp
Original file line number Diff line number Diff line change
@@ -2501,8 +2501,9 @@ swift::swift_getExistentialTypeMetadata(ProtocolClassConstraint classConstraint,
const ProtocolDescriptor **protocols)
SWIFT_CC(RegisterPreservingCC_IMPL) {

// Sort the protocol set.
std::sort(protocols, protocols + numProtocols);
// We entrust that the compiler emitting the call to
// swift_getExistentialTypeMetadata always sorts the `protocols` array using
// a globally stable ordering that's consistent across modules.

ExistentialCacheEntry::Key key = {
superclassConstraint, classConstraint, numProtocols, protocols
41 changes: 2 additions & 39 deletions unittests/runtime/Metadata.cpp
Original file line number Diff line number Diff line change
@@ -449,41 +449,6 @@ TEST(MetadataTest, getExistentialMetadata) {
return b;
});

// protocol compositions are order-invariant
RaceTest_ExpectEqual<const ExistentialTypeMetadata *>(
[&]() -> const ExistentialTypeMetadata * {

const ProtocolDescriptor *protoList4[] = {
&ProtocolA,
&ProtocolB
};

const ProtocolDescriptor *protoList5[] = {
&ProtocolB,
&ProtocolA
};

auto ab = swift_getExistentialTypeMetadata(ProtocolClassConstraint::Any,
/*superclass=*/nullptr,
2, protoList4);
auto ba = swift_getExistentialTypeMetadata(ProtocolClassConstraint::Any,
/*superclass=*/nullptr,
2, protoList5);
EXPECT_EQ(ab, ba);
EXPECT_EQ(MetadataKind::Existential, ab->getKind());
EXPECT_EQ(2U, ab->Flags.getNumWitnessTables());
EXPECT_EQ(ProtocolClassConstraint::Any, ab->Flags.getClassConstraint());
EXPECT_EQ(2U, ab->Protocols.NumProtocols);
EXPECT_TRUE(
(ab->Protocols[0]==&ProtocolA && ab->Protocols[1]==&ProtocolB)
|| (ab->Protocols[0]==&ProtocolB && ab->Protocols[1]==&ProtocolA));
EXPECT_EQ(SpecialProtocol::None,
ab->Flags.getSpecialProtocol());
EXPECT_EQ(nullptr,
ab->getSuperclassConstraint());
return ab;
});

const ProtocolDescriptor *protoList6[] = {
&ProtocolClassConstrained,
};
@@ -865,10 +830,8 @@ TEST(MetadataTest, getExistentialTypeMetadata_subclass) {
EXPECT_EQ(ProtocolClassConstraint::Class,
ex2->Flags.getClassConstraint());
EXPECT_EQ(2U, ex2->Protocols.NumProtocols);
EXPECT_TRUE((ex2->Protocols[0] == &OpaqueProto1 &&
ex2->Protocols[1] == &ClassProto1) ||
(ex2->Protocols[0] == &ClassProto1 &&
ex2->Protocols[1] == &OpaqueProto1));
EXPECT_TRUE(ex2->Protocols[0] == &OpaqueProto1 &&
ex2->Protocols[1] == &ClassProto1);
EXPECT_EQ(&MetadataTest2, ex2->getSuperclassConstraint());
return ex2;
});

0 comments on commit 36127b2

Please sign in to comment.