Skip to content

Commit e3d2f75

Browse files
slavapestovslavapestov
authored
Merge pull request #77632 from slavapestov/subst-map-verify
Flush out incorrectly-constructed substitution maps
2 parents 1016971 + 080f3d2 commit e3d2f75

14 files changed

+183
-124
lines changed

‎include/swift/AST/SubstitutionMap.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,9 @@ class SubstitutionMap {
213213
/// interface types.
214214
SubstitutionMap mapReplacementTypesOutOfContext() const;
215215

216-
/// Verify that this substitution map is valid.
217-
void verify() const;
216+
/// Verify that the conformances stored in this substitution map match the
217+
/// replacement types provided.
218+
void verify(bool allowInvalid=true) const;
218219

219220
/// Whether to dump the full substitution map, or just a minimal useful subset
220221
/// (on a single line).

‎lib/AST/ProtocolConformance.cpp

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -976,8 +976,7 @@ ProtocolConformance::subst(InFlightSubstitution &IFS) const {
976976
switch (getKind()) {
977977
case ProtocolConformanceKind::Normal: {
978978
auto origType = getType();
979-
if (!origType->hasTypeParameter() &&
980-
!origType->hasArchetype())
979+
if (IFS.isInvariant(origType))
981980
return ProtocolConformanceRef(mutableThis);
982981

983982
auto substType = origType.subst(IFS);
@@ -998,8 +997,7 @@ ProtocolConformance::subst(InFlightSubstitution &IFS) const {
998997

999998
case ProtocolConformanceKind::Builtin: {
1000999
auto origType = getType();
1001-
if (!origType->hasTypeParameter() &&
1002-
!origType->hasArchetype())
1000+
if (IFS.isInvariant(origType))
10031001
return ProtocolConformanceRef(mutableThis);
10041002

10051003
auto substType = origType.subst(IFS);
@@ -1022,18 +1020,14 @@ ProtocolConformance::subst(InFlightSubstitution &IFS) const {
10221020

10231021
case ProtocolConformanceKind::Inherited: {
10241022
// Substitute the base.
1025-
auto inheritedConformance
1026-
= cast<InheritedProtocolConformance>(this)->getInheritedConformance();
1027-
10281023
auto origType = getType();
1029-
if (!origType->hasTypeParameter() &&
1030-
!origType->hasArchetype()) {
1024+
if (IFS.isInvariant(origType))
10311025
return ProtocolConformanceRef(mutableThis);
1032-
}
10331026

1034-
auto origBaseType = inheritedConformance->getType();
1035-
if (origBaseType->hasTypeParameter() ||
1036-
origBaseType->hasArchetype()) {
1027+
auto inheritedConformance
1028+
= cast<InheritedProtocolConformance>(this)->getInheritedConformance();
1029+
1030+
if (!IFS.isInvariant(inheritedConformance->getType())) {
10371031
// Substitute into the superclass.
10381032
auto substConformance = inheritedConformance->subst(IFS);
10391033
if (!substConformance.isConcrete())

‎lib/AST/RequirementEnvironment.cpp

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -118,28 +118,41 @@ RequirementEnvironment::RequirementEnvironment(
118118
}
119119
return substGenericParam;
120120
},
121-
[selfType, substConcreteType, conformance, conformanceDC, &ctx](
121+
[selfType, substConcreteType, conformance, conformanceDC, covariantSelf, &ctx](
122122
CanType type, Type replacement, ProtocolDecl *proto)
123123
-> ProtocolConformanceRef {
124124
// The protocol 'Self' conforms concretely to the conforming type.
125125
if (type->isEqual(selfType)) {
126-
ProtocolConformance *specialized = conformance;
127-
if (conformance && conformance->getGenericSignature()) {
128-
auto concreteSubs =
129-
substConcreteType->getContextSubstitutionMap(conformanceDC);
130-
specialized =
131-
ctx.getSpecializedConformance(substConcreteType,
132-
cast<NormalProtocolConformance>(conformance),
133-
concreteSubs);
134-
}
126+
ASSERT(covariantSelf || replacement->isEqual(substConcreteType));
127+
128+
if (conformance) {
129+
ProtocolConformance *specialized = conformance;
130+
131+
if (conformance->getGenericSignature()) {
132+
auto concreteSubs =
133+
substConcreteType->getContextSubstitutionMap(conformanceDC);
134+
specialized =
135+
ctx.getSpecializedConformance(substConcreteType,
136+
cast<NormalProtocolConformance>(conformance),
137+
concreteSubs);
138+
}
139+
140+
// findWitnessedObjCRequirements() does a weird thing by passing in a
141+
// DC that is not the conformance DC. Work around it here.
142+
if (!specialized->getType()->isEqual(replacement)) {
143+
ASSERT(specialized->getType()->isExactSuperclassOf(substConcreteType));
144+
ASSERT(covariantSelf ? replacement->is<GenericTypeParamType>()
145+
: replacement->isEqual(substConcreteType));
146+
specialized = ctx.getInheritedConformance(replacement, specialized);
147+
}
135148

136-
if (specialized)
137149
return ProtocolConformanceRef(specialized);
150+
}
138151
}
139152

140153
// All other generic parameters come from the requirement itself
141154
// and conform abstractly.
142-
return ProtocolConformanceRef(proto);
155+
return MakeAbstractConformanceForGenericType()(type, replacement, proto);
143156
});
144157

145158
// If the requirement itself is non-generic, the witness thunk signature

‎lib/AST/SubstitutionMap.cpp

Lines changed: 82 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,8 @@ SubstitutionMap::SubstitutionMap(
6060
ArrayRef<Type> replacementTypes,
6161
ArrayRef<ProtocolConformanceRef> conformances)
6262
: storage(Storage::get(genericSig, replacementTypes, conformances)) {
63-
#ifndef NDEBUG
6463
if (genericSig->getASTContext().LangOpts.VerifyAllSubstitutionMaps)
6564
verify();
66-
#endif
6765
}
6866

6967
ArrayRef<ProtocolConformanceRef> SubstitutionMap::getConformances() const {
@@ -534,78 +532,109 @@ SubstitutionMap::getOverrideSubstitutions(const NominalTypeDecl *baseNominal,
534532
LookUpConformanceInOverrideSubs(info));
535533
}
536534

537-
void SubstitutionMap::verify() const {
538-
#ifndef NDEBUG
535+
void SubstitutionMap::verify(bool allowInvalid) const {
539536
if (empty())
540537
return;
541538

539+
auto genericSig = getGenericSignature();
540+
auto &ctx = genericSig->getASTContext();
541+
542+
if (ctx.isRecursivelyConstructingRequirementMachine(
543+
genericSig.getCanonicalSignature()))
544+
return;
545+
542546
unsigned conformanceIndex = 0;
543547

544-
for (const auto &req : getGenericSignature().getRequirements()) {
548+
for (const auto &req : genericSig.getRequirements()) {
545549
if (req.getKind() != RequirementKind::Conformance)
546550
continue;
547551

548552
SWIFT_DEFER { ++conformanceIndex; };
553+
auto conformance = getConformances()[conformanceIndex];
554+
549555
auto substType = req.getFirstType().subst(*this);
550-
if (substType->isTypeParameter() ||
551-
substType->is<ArchetypeType>() ||
552-
substType->isTypeVariableOrMember() ||
553-
substType->is<UnresolvedType>() ||
554-
substType->hasError())
555-
continue;
556556

557-
auto conformance = getConformances()[conformanceIndex];
557+
// Unwrap various strange things.
558+
substType = substType->getReferenceStorageReferent();
559+
if (auto *selfType = substType->getAs<DynamicSelfType>())
560+
substType = selfType->getSelfType();
558561

559-
if (conformance.isInvalid())
560-
continue;
562+
// Don't bother validating these cases.
563+
if (allowInvalid && substType->hasUnboundGenericType())
564+
return;
565+
566+
if (conformance.isInvalid()) {
567+
if (!allowInvalid) {
568+
llvm::errs() << "Unexpected invalid conformance in substitution map:\n";
569+
dump(llvm::dbgs());
570+
llvm::errs() << "\n";
571+
abort();
572+
}
561573

562-
// All of the conformances should be concrete.
563-
if (!conformance.isConcrete()) {
564-
llvm::dbgs() << "Concrete type cannot have abstract conformance:\n";
565-
substType->dump(llvm::dbgs());
566-
llvm::dbgs() << "SubstitutionMap:\n";
567-
dump(llvm::dbgs());
568-
llvm::dbgs() << "\n";
569-
llvm::dbgs() << "Requirement:\n";
570-
req.dump(llvm::dbgs());
571-
llvm::dbgs() << "\n";
574+
continue;
572575
}
573-
assert(conformance.isConcrete() && "Conformance should be concrete");
574-
575-
if (substType->is<UnboundGenericType>())
576+
577+
if (conformance.isAbstract()) {
578+
if (!substType->isTypeParameter() &&
579+
!substType->is<PackElementType>() &&
580+
!substType->is<ArchetypeType>() &&
581+
!substType->isTypeVariableOrMember() &&
582+
!substType->is<UnresolvedType>() &&
583+
!substType->is<PlaceholderType>() &&
584+
!substType->is<ErrorType>()) {
585+
llvm::errs() << "Unexpected abstract conformance in substitution map:\n";
586+
dump(llvm::errs());
587+
llvm::errs() << "\n";
588+
abort();
589+
}
590+
576591
continue;
577-
578-
auto conformanceTy = conformance.getConcrete()->getType();
579-
if (conformanceTy->hasTypeParameter()
580-
&& !substType->hasTypeParameter()) {
581-
conformanceTy = conformance.getConcrete()->getDeclContext()
582-
->mapTypeIntoContext(conformanceTy);
583592
}
584-
585-
if (!substType->isEqual(conformanceTy)) {
586-
llvm::dbgs() << "Conformance must match concrete replacement type:\n";
587-
substType->dump(llvm::dbgs());
588-
llvm::dbgs() << "Conformance type:\n";
589-
conformance.getConcrete()->getType()->dump(llvm::dbgs());
590-
llvm::dbgs() << "Conformance:\n";
591-
conformance.dump(llvm::dbgs());
592-
llvm::dbgs() << "\n";
593-
llvm::dbgs() << "SubstitutionMap:\n";
594-
dump(llvm::dbgs());
595-
llvm::dbgs() << "\n";
596-
llvm::dbgs() << "Requirement:\n";
597-
req.dump(llvm::dbgs());
598-
llvm::dbgs() << "\n";
593+
594+
if (conformance.isPack()) {
595+
// FIXME: Implement some kind of check here.
596+
continue;
599597
}
600-
assert(substType->isEqual(conformanceTy)
601-
&& "conformance should match corresponding type");
598+
599+
auto *concrete = conformance.getConcrete();
602600

603601
if (substType->isExistentialType()) {
604-
assert(isa<SelfProtocolConformance>(conformance.getConcrete()) &&
605-
"Existential type cannot have normal conformance");
602+
if (req.getProtocolDecl()->isSpecificProtocol(KnownProtocolKind::Sendable) &&
603+
isa<BuiltinProtocolConformance>(concrete)) {
604+
continue;
605+
}
606+
607+
if (!isa<SelfProtocolConformance>(concrete)) {
608+
// A superclass-constrained self-conforming existential might conform
609+
// concretely.
610+
if (substType->getSuperclass())
611+
continue;
612+
613+
llvm::errs() << "Expected to find a self conformance:\n";
614+
substType->dump(llvm::errs());
615+
llvm::errs() << "Substitution map:\n";
616+
dump(llvm::errs());
617+
llvm::errs() << "\n";
618+
abort();
619+
}
620+
621+
continue;
622+
}
623+
624+
if (substType->isTypeParameter())
625+
continue;
626+
627+
if (!concrete->getType()->isEqual(substType)) {
628+
llvm::errs() << "Conformance with wrong conforming type:\n";
629+
concrete->getType()->dump(llvm::errs());
630+
llvm::errs() << "Should be:\n";
631+
substType->dump(llvm::errs());
632+
llvm::errs() << "Substitution map:\n";
633+
dump(llvm::errs());
634+
llvm::errs() << "\n";
635+
abort();
606636
}
607637
}
608-
#endif
609638
}
610639

611640
void SubstitutionMap::profile(llvm::FoldingSetNodeID &id) const {

‎lib/SILGen/ResultPlan.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,9 @@ class ForeignAsyncInitializationPlan final : public ResultPlan {
772772
auto createIntrinsic =
773773
throws ? SGF.SGM.getCreateCheckedThrowingContinuation()
774774
: SGF.SGM.getCreateCheckedContinuation();
775+
auto conformances =
776+
collectExistentialConformances(calleeTypeInfo.substResultType,
777+
ctx.TheAnyType);
775778
auto subs =
776779
SubstitutionMap::get(createIntrinsic->getGenericSignature(),
777780
{calleeTypeInfo.substResultType}, conformances);

‎lib/SILGen/SILGenPoly.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5438,7 +5438,10 @@ CanSILFunctionType SILGenFunction::buildThunkType(
54385438
SubstitutionMap &interfaceSubs,
54395439
CanType &dynamicSelfType,
54405440
bool withoutActuallyEscaping) {
5441-
return buildSILFunctionThunkType(&F, sourceType, expectedType, inputSubstType, outputSubstType, genericEnv, interfaceSubs, dynamicSelfType, withoutActuallyEscaping);
5441+
return buildSILFunctionThunkType(
5442+
&F, sourceType, expectedType, inputSubstType, outputSubstType,
5443+
genericEnv, interfaceSubs, dynamicSelfType,
5444+
withoutActuallyEscaping);
54425445
}
54435446

54445447
static ManagedValue createPartialApplyOfThunk(SILGenFunction &SGF,
@@ -7227,8 +7230,7 @@ void SILGenFunction::emitProtocolWitness(
72277230
auto deallocCleanup = std::get<3>(tokenAndCleanups);
72287231

72297232
YieldInfo witnessYieldInfo(SGM, witness, witnessFTy, witnessSubs);
7230-
YieldInfo reqtYieldInfo(SGM, requirement, thunkTy,
7231-
reqtSubs.subst(getForwardingSubstitutionMap()));
7233+
YieldInfo reqtYieldInfo(SGM, requirement, thunkTy, reqtSubs);
72327234

72337235
translateYields(*this, loc, witnessYields, witnessYieldInfo, reqtYieldInfo);
72347236

‎lib/SILGen/SILGenType.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -752,13 +752,6 @@ SILFunction *SILGenModule::emitProtocolWitness(
752752
reqtOrigTy->substGenericArgs(reqtSubMap)
753753
->getReducedType(genericSig));
754754

755-
// Generic signatures where all parameters are concrete are lowered away
756-
// at the SILFunctionType level.
757-
if (genericSig && genericSig->areAllParamsConcrete()) {
758-
genericSig = nullptr;
759-
genericEnv = nullptr;
760-
}
761-
762755
// Rewrite the conformance in terms of the requirement environment's Self
763756
// type, which might have a different generic signature than the type
764757
// itself.
@@ -772,6 +765,16 @@ SILFunction *SILGenModule::emitProtocolWitness(
772765
auto self = requirement->getSelfInterfaceType()->getCanonicalType();
773766

774767
conformance = reqtSubMap.lookupConformance(self, requirement);
768+
769+
if (genericEnv)
770+
reqtSubMap = reqtSubMap.subst(genericEnv->getForwardingSubstitutionMap());
771+
}
772+
773+
// Generic signatures where all parameters are concrete are lowered away
774+
// at the SILFunctionType level.
775+
if (genericSig && genericSig->areAllParamsConcrete()) {
776+
genericSig = nullptr;
777+
genericEnv = nullptr;
775778
}
776779

777780
reqtSubstTy =
@@ -794,6 +797,15 @@ SILFunction *SILGenModule::emitProtocolWitness(
794797
}
795798
}
796799

800+
ProtocolConformance *manglingConformance = nullptr;
801+
if (conformance.isConcrete()) {
802+
manglingConformance = conformance.getConcrete();
803+
if (auto *inherited = dyn_cast<InheritedProtocolConformance>(manglingConformance)) {
804+
manglingConformance = inherited->getInheritedConformance();
805+
conformance = ProtocolConformanceRef(manglingConformance);
806+
}
807+
}
808+
797809
// Lower the witness thunk type with the requirement's abstraction level.
798810
auto witnessSILFnType = getNativeSILFunctionType(
799811
M.Types, TypeExpansionContext::minimal(), AbstractionPattern(reqtOrigTy),
@@ -802,8 +814,6 @@ SILFunction *SILGenModule::emitProtocolWitness(
802814

803815
// Mangle the name of the witness thunk.
804816
Mangle::ASTMangler NewMangler;
805-
auto manglingConformance =
806-
conformance.isConcrete() ? conformance.getConcrete() : nullptr;
807817
std::string nameBuffer =
808818
NewMangler.mangleWitnessThunk(manglingConformance, requirement.getDecl());
809819
// TODO(TF-685): Proper mangling for derivative witness thunks.

‎lib/SILOptimizer/Utils/Generics.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3137,7 +3137,7 @@ static bool usePrespecialized(
31373137

31383138
auto newSubstMap = SubstitutionMap::get(
31393139
apply.getSubstitutionMap().getGenericSignature(), newSubs,
3140-
apply.getSubstitutionMap().getConformances());
3140+
LookUpConformanceInModule());
31413141

31423142
ReabstractionInfo layoutReInfo = ReabstractionInfo(
31433143
funcBuilder.getModule().getSwiftModule(),

0 commit comments

Comments
 (0)