Skip to content

Commit 2d1fc68

Browse files
committed
[SIL] Eliminate SILFunctionType::getDefaultWitnessMethodProtocol().
This method wasn’t returning the protocol on which the that the witness method would satisfy, as documented. Rather, it was returning the protocol to which the `Self` type conforms, which could be completely unrelated. For example, in IndexingIterator’s conformance to IteratorProtocol, this method would produce the protocol “Collection”, because that’s where the witness itself was implemented. However, there isn’t necessarily a single such protocol, so checking for/returning a single protocol was incorrect. It turns out that there were only a few SIL verifier assertions of it (that are trivially true) and two actual uses in code: (1) The devirtualizer was using this computation to decide when it didn’t need to perform any additional substitutions, but it’s predicate for doing so was essentially incorrect. Instead, it really wanted to check whether the Self type is still a type parameter. (2) Our polymorphic convention was using it to essentially check whether the ’Self’ instance type of a witness_method was a GenericTypeParamType, which we can check directly. Fixes rdar://problem/47767506 and possibly the hard-to-reproduce rdar://problem/47772899.
1 parent 0d36a1f commit 2d1fc68

File tree

6 files changed

+9
-54
lines changed

6 files changed

+9
-54
lines changed

include/swift/AST/Types.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4007,11 +4007,6 @@ class SILFunctionType final : public TypeBase, public llvm::FoldingSetNode,
40074007

40084008
CanType getSelfInstanceType() const;
40094009

4010-
/// If this is a @convention(witness_method) function with a protocol
4011-
/// constrained self parameter, return the protocol constraint for
4012-
/// the Self type.
4013-
ProtocolDecl *getDefaultWitnessMethodProtocol() const;
4014-
40154010
/// If this is a @convention(witness_method) function with a class
40164011
/// constrained self parameter, return the class constraint for the
40174012
/// Self type.

lib/IRGen/GenProto.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,8 +329,7 @@ void PolymorphicConvention::considerWitnessSelf(CanSILFunctionType fnType) {
329329
MetadataSource::InvalidSourceIndex,
330330
selfTy);
331331

332-
if (fnType->getDefaultWitnessMethodProtocol() ||
333-
fnType->getWitnessMethodClass(M)) {
332+
if (fnType->getSelfInstanceType()->is<GenericTypeParamType>()) {
334333
// The Self type is abstract, so we can fulfill its metadata from
335334
// the Self metadata parameter.
336335
addSelfMetadataFulfillment(selfTy);

lib/SIL/SILFunctionType.cpp

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,6 @@ CanType SILFunctionType::getSelfInstanceType() const {
9898
return selfTy;
9999
}
100100

101-
ProtocolDecl *
102-
SILFunctionType::getDefaultWitnessMethodProtocol() const {
103-
assert(getRepresentation() == SILFunctionTypeRepresentation::WitnessMethod);
104-
auto selfTy = getSelfInstanceType();
105-
if (auto paramTy = dyn_cast<GenericTypeParamType>(selfTy)) {
106-
assert(paramTy->getDepth() == 0 && paramTy->getIndex() == 0);
107-
auto superclass = GenericSig->getSuperclassBound(paramTy);
108-
if (superclass)
109-
return nullptr;
110-
111-
assert(!GenericSig->getRequirements().empty());
112-
assert(GenericSig->getRequirements().front().getKind() ==
113-
RequirementKind::Conformance);
114-
assert(GenericSig->getRequirements().front().getFirstType()
115-
->isEqual(paramTy));
116-
return GenericSig->getRequirements().front().getSecondType()
117-
->castTo<ProtocolType>()->getDecl();
118-
}
119-
120-
return nullptr;
121-
}
122-
123101
ClassDecl *
124102
SILFunctionType::getWitnessMethodClass(ModuleDecl &M) const {
125103
auto selfTy = getSelfInstanceType();

lib/SIL/SILVerifier.cpp

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5027,8 +5027,6 @@ void SILWitnessTable::verify(const SILModule &M) const {
50275027
assert(getEntries().empty() &&
50285028
"A witness table declaration should not have any entries.");
50295029

5030-
auto *protocol = getConformance()->getProtocol();
5031-
50325030
for (const Entry &E : getEntries())
50335031
if (E.getKind() == SILWitnessTable::WitnessKind::Method) {
50345032
SILFunction *F = E.getMethodWitness().Witness;
@@ -5044,15 +5042,6 @@ void SILWitnessTable::verify(const SILModule &M) const {
50445042
assert(F->getLoweredFunctionType()->getRepresentation() ==
50455043
SILFunctionTypeRepresentation::WitnessMethod &&
50465044
"Witnesses must have witness_method representation.");
5047-
auto *witnessSelfProtocol = F->getLoweredFunctionType()
5048-
->getDefaultWitnessMethodProtocol();
5049-
assert((witnessSelfProtocol == nullptr ||
5050-
witnessSelfProtocol == protocol) &&
5051-
"Witnesses must either have a concrete Self, or an "
5052-
"an abstract Self that is constrained to their "
5053-
"protocol.");
5054-
(void)protocol;
5055-
(void)witnessSelfProtocol;
50565045
}
50575046
}
50585047
}
@@ -5077,12 +5066,6 @@ void SILDefaultWitnessTable::verify(const SILModule &M) const {
50775066
assert(F->getLoweredFunctionType()->getRepresentation() ==
50785067
SILFunctionTypeRepresentation::WitnessMethod &&
50795068
"Default witnesses must have witness_method representation.");
5080-
5081-
auto *witnessSelfProtocol = F->getLoweredFunctionType()
5082-
->getDefaultWitnessMethodProtocol();
5083-
assert(witnessSelfProtocol == getProtocol() &&
5084-
"Default witnesses must have an abstract Self parameter "
5085-
"constrained to their protocol.");
50865069
}
50875070
#endif
50885071
}

lib/SILOptimizer/Utils/Devirtualize.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,8 @@ swift::tryDevirtualizeClassMethod(FullApplySite AI, SILValue ClassInstance,
870870
/// \param requirementSig The generic signature of the requirement
871871
/// \param witnessThunkSig The generic signature of the witness method
872872
/// \param origSubMap The substitutions from the call instruction
873-
/// \param isDefaultWitness True if this is a default witness method
873+
/// \param isSelfAbstract True if the Self type of the witness method is
874+
/// still abstract (i.e., not a concrete type).
874875
/// \param classWitness The ClassDecl if this is a class witness method
875876
static SubstitutionMap
876877
getWitnessMethodSubstitutions(
@@ -879,13 +880,13 @@ getWitnessMethodSubstitutions(
879880
GenericSignature *requirementSig,
880881
GenericSignature *witnessThunkSig,
881882
SubstitutionMap origSubMap,
882-
bool isDefaultWitness,
883+
bool isSelfAbstract,
883884
ClassDecl *classWitness) {
884885

885886
if (witnessThunkSig == nullptr)
886887
return SubstitutionMap();
887888

888-
if (isDefaultWitness)
889+
if (isSelfAbstract && !classWitness)
889890
return origSubMap;
890891

891892
assert(!conformanceRef.isAbstract());
@@ -947,14 +948,13 @@ swift::getWitnessMethodSubstitutions(SILModule &Module, ApplySite AI,
947948
SubstitutionMap origSubs = AI.getSubstitutionMap();
948949

949950
auto *mod = Module.getSwiftModule();
950-
bool isDefaultWitness =
951-
(witnessFnTy->getDefaultWitnessMethodProtocol()
952-
== CRef.getRequirement());
951+
bool isSelfAbstract =
952+
witnessFnTy->getSelfInstanceType()->is<GenericTypeParamType>();
953953
auto *classWitness = witnessFnTy->getWitnessMethodClass(*mod);
954954

955955
return ::getWitnessMethodSubstitutions(mod, CRef, requirementSig,
956956
witnessThunkSig, origSubs,
957-
isDefaultWitness, classWitness);
957+
isSelfAbstract, classWitness);
958958
}
959959

960960
/// Generate a new apply of a function_ref to replace an apply of a

test/IRGen/witness_method_default.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public protocol SIMDScalarStub {
1313
func abs() -> Self
1414
}
1515

16-
// CHECK: define swiftcc void @"$s22witness_method_default7callAbs1sxx_tAA14SIMDScalarStubRzlF
16+
// CHECK: define {{.*}}swiftcc void @"$s22witness_method_default7callAbs1sxx_tAA14SIMDScalarStubRzlF
1717
public func callAbs<T: SIMDScalarStub>(s: T) -> T {
1818
// CHECK: [[ABS_PTR:%[0-9]+]] = getelementptr inbounds i8*, i8** %T.SIMDScalarStub, i32 3
1919
// CHECK-NEXT: [[ABS_VALUE:%[0-9]+]] = load i8*, i8** [[ABS_PTR]]

0 commit comments

Comments
 (0)