Skip to content

Commit b566a3d

Browse files
dan-zhengdan-zheng
committed
NFC: Add GenericSignature::getCanonicalSignature.
Motivation: `GenericSignatureImpl::getCanonicalSignature` crashes for `GenericSignature` with underlying `nullptr`. This led to verbose workarounds when computing `CanGenericSignature` from `GenericSignature`. Solution: `GenericSignature::getCanonicalSignature` is a wrapper around `GenericSignatureImpl::getCanonicalSignature` that returns the canonical signature, or `nullptr` if the underlying pointer is `nullptr`. Rewrite all verbose workarounds using `GenericSignature::getCanonicalSignature`.
1 parent 8b7ce27 commit b566a3d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+119
-122
lines changed

‎include/swift/AST/GenericSignature.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class ConformanceAccessPath {
9393
SWIFT_DEBUG_DUMP;
9494
};
9595

96+
class CanGenericSignature;
9697
class GenericSignatureImpl;
9798
class GenericTypeParamType;
9899

@@ -140,6 +141,10 @@ class GenericSignature {
140141
SWIFT_DEBUG_DUMP;
141142
std::string getAsString() const;
142143

144+
/// Returns the canonical generic signature, or \c nullptr if the underlying
145+
/// pointer is \c nullptr. The result is cached.
146+
CanGenericSignature getCanonicalSignature() const;
147+
143148
// Support for FoldingSet.
144149
void Profile(llvm::FoldingSetNodeID &id) const;
145150

@@ -300,8 +305,8 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
300305
bool isCanonical() const;
301306

302307
ASTContext &getASTContext() const;
303-
304-
/// Canonicalize the components of a generic signature.
308+
309+
/// Returns the canonical generic signature. The result is cached.
305310
CanGenericSignature getCanonicalSignature() const;
306311

307312
/// Retrieve the generic signature builder for the given generic signature.

‎lib/AST/ASTContext.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,7 +1525,7 @@ static AllocationArena getArena(GenericSignature genericSig) {
15251525
void ASTContext::registerGenericSignatureBuilder(
15261526
GenericSignature sig,
15271527
GenericSignatureBuilder &&builder) {
1528-
auto canSig = sig->getCanonicalSignature();
1528+
auto canSig = sig.getCanonicalSignature();
15291529
auto arena = getArena(sig);
15301530
auto &genericSignatureBuilders =
15311531
getImpl().getArena(arena).GenericSignatureBuilders;
@@ -1564,12 +1564,12 @@ GenericSignatureBuilder *ASTContext::getOrCreateGenericSignatureBuilder(
15641564
auto builderSig =
15651565
builder->computeGenericSignature(SourceLoc(),
15661566
/*allowConcreteGenericParams=*/true);
1567-
if (builderSig->getCanonicalSignature() != sig) {
1567+
if (builderSig.getCanonicalSignature() != sig) {
15681568
llvm::errs() << "ERROR: generic signature builder is not idempotent.\n";
15691569
llvm::errs() << "Original generic signature : ";
15701570
sig->print(llvm::errs());
15711571
llvm::errs() << "\nReprocessed generic signature: ";
1572-
auto reprocessedSig = builderSig->getCanonicalSignature();
1572+
auto reprocessedSig = builderSig.getCanonicalSignature();
15731573

15741574
reprocessedSig->print(llvm::errs());
15751575
llvm::errs() << "\n";
@@ -3277,8 +3277,8 @@ SILFunctionType::SILFunctionType(
32773277
"types and substitutions");
32783278

32793279
if (substitutions) {
3280-
assert(substitutions.getGenericSignature()->getCanonicalSignature()
3281-
== genericSig->getCanonicalSignature()
3280+
assert(substitutions.getGenericSignature().getCanonicalSignature()
3281+
== genericSig.getCanonicalSignature()
32823282
&& "substitutions must match generic signature");
32833283
}
32843284

‎lib/AST/ASTDemangler.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ Type ASTBuilder::createImplFunctionType(
454454
ArrayRef<Demangle::ImplFunctionResult<Type>> results,
455455
Optional<Demangle::ImplFunctionResult<Type>> errorResult,
456456
ImplFunctionTypeFlags flags) {
457-
GenericSignature genericSig = GenericSignature();
457+
GenericSignature genericSig;
458458

459459
SILCoroutineKind funcCoroutineKind = SILCoroutineKind::None;
460460
ParameterConvention funcCalleeConvention =
@@ -870,7 +870,7 @@ CanGenericSignature ASTBuilder::demangleGenericSignature(
870870
AbstractGenericSignatureRequest{
871871
nominalDecl->getGenericSignature().getPointer(), { },
872872
std::move(requirements)},
873-
GenericSignature())->getCanonicalSignature();
873+
GenericSignature()).getCanonicalSignature();
874874
}
875875

876876
DeclContext *
@@ -980,7 +980,7 @@ ASTBuilder::findDeclContext(NodePointer node) {
980980
continue;
981981
}
982982

983-
if (ext->getGenericSignature()->getCanonicalSignature()
983+
if (ext->getGenericSignature().getCanonicalSignature()
984984
== genericSig) {
985985
return ext;
986986
}

‎lib/AST/ASTMangler.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ std::string ASTMangler::mangleReabstractionThunkHelper(
360360
assert(ThunkType->getSubstitutions().empty() && "not implemented");
361361
GenericSignature GenSig = ThunkType->getSubstGenericSignature();
362362
if (GenSig)
363-
CurGenericSignature = GenSig->getCanonicalSignature();
363+
CurGenericSignature = GenSig.getCanonicalSignature();
364364

365365
beginMangling();
366366
appendType(FromType);
@@ -1199,7 +1199,7 @@ void ASTMangler::bindGenericParameters(CanGenericSignature sig) {
11991199
/// Bind the generic parameters from the given context and its parents.
12001200
void ASTMangler::bindGenericParameters(const DeclContext *DC) {
12011201
if (auto sig = DC->getGenericSignatureOfContext())
1202-
bindGenericParameters(sig->getCanonicalSignature());
1202+
bindGenericParameters(sig.getCanonicalSignature());
12031203
}
12041204

12051205
void ASTMangler::appendFlatGenericArgs(SubstitutionMap subs) {
@@ -2121,7 +2121,7 @@ void ASTMangler::appendTypeListElement(Identifier name, Type elementType,
21212121

21222122
bool ASTMangler::appendGenericSignature(GenericSignature sig,
21232123
GenericSignature contextSig) {
2124-
auto canSig = sig->getCanonicalSignature();
2124+
auto canSig = sig.getCanonicalSignature();
21252125
CurGenericSignature = canSig;
21262126

21272127
unsigned initialParamDepth;
@@ -2131,7 +2131,7 @@ bool ASTMangler::appendGenericSignature(GenericSignature sig,
21312131
if (contextSig) {
21322132
// If the signature is the same as the context signature, there's nothing
21332133
// to do.
2134-
if (contextSig->getCanonicalSignature() == canSig) {
2134+
if (contextSig.getCanonicalSignature() == canSig) {
21352135
return false;
21362136
}
21372137

@@ -2442,8 +2442,8 @@ CanType ASTMangler::getDeclTypeForMangling(
24422442

24432443
void ASTMangler::appendDeclType(const ValueDecl *decl, bool isFunctionMangling) {
24442444
Mod = decl->getModuleContext();
2445-
GenericSignature genericSig = GenericSignature();
2446-
GenericSignature parentGenericSig = GenericSignature();
2445+
GenericSignature genericSig;
2446+
GenericSignature parentGenericSig;
24472447
auto type = getDeclTypeForMangling(decl, genericSig, parentGenericSig);
24482448

24492449
if (AnyFunctionType *FuncTy = type->getAs<AnyFunctionType>()) {
@@ -2559,7 +2559,7 @@ void ASTMangler::appendEntity(const ValueDecl *decl) {
25592559

25602560
void
25612561
ASTMangler::appendProtocolConformance(const ProtocolConformance *conformance) {
2562-
GenericSignature contextSig = GenericSignature();
2562+
GenericSignature contextSig;
25632563
auto topLevelContext =
25642564
conformance->getDeclContext()->getModuleScopeContext();
25652565
Mod = topLevelContext->getParentModule();
@@ -2702,7 +2702,7 @@ void ASTMangler::appendConcreteProtocolConformance(
27022702
auto shouldUseConformanceSig = !CurGenericSignature && conformanceSig;
27032703
llvm::SaveAndRestore<CanGenericSignature> savedSignature(
27042704
CurGenericSignature, shouldUseConformanceSig
2705-
? conformanceSig->getCanonicalSignature()
2705+
? conformanceSig.getCanonicalSignature()
27062706
: CurGenericSignature);
27072707

27082708
// Conforming type.
@@ -2745,7 +2745,7 @@ void ASTMangler::appendConcreteProtocolConformance(
27452745
// Append the conformance access path with the signature of the opaque type.
27462746
{
27472747
llvm::SaveAndRestore<CanGenericSignature> savedSignature(
2748-
CurGenericSignature, opaqueSignature->getCanonicalSignature());
2748+
CurGenericSignature, opaqueSignature.getCanonicalSignature());
27492749
appendDependentProtocolConformance(conformanceAccessPath);
27502750
}
27512751
appendType(canType);

‎lib/AST/GenericSignature.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,12 @@ GenericSignatureBuilder *GenericSignatureImpl::getGenericSignatureBuilder() {
167167
}
168168

169169
bool GenericSignatureImpl::isEqual(GenericSignature Other) {
170-
return getCanonicalSignature() == Other.getPointer()->getCanonicalSignature();
170+
return getCanonicalSignature() == Other.getCanonicalSignature();
171171
}
172172

173173
bool GenericSignatureImpl::isCanonical() const {
174-
if (CanonicalSignatureOrASTContext.is<ASTContext*>()) return true;
175-
174+
if (CanonicalSignatureOrASTContext.is<ASTContext*>())
175+
return true;
176176
return getCanonicalSignature().getPointer() == this;
177177
}
178178

@@ -310,6 +310,14 @@ CanGenericSignature::getCanonical(TypeArrayView<GenericTypeParamType> params,
310310
return CanGenericSignature(canSig);
311311
}
312312

313+
CanGenericSignature GenericSignature::getCanonicalSignature() const {
314+
// If the underlying pointer is null, return `CanGenericSignature()`.
315+
if (isNull())
316+
return CanGenericSignature();
317+
// Otherwise, return the canonical signature of the underlying pointer.
318+
return getPointer()->getCanonicalSignature();
319+
}
320+
313321
CanGenericSignature GenericSignatureImpl::getCanonicalSignature() const {
314322
// If we haven't computed the canonical signature yet, do so now.
315323
if (CanonicalSignatureOrASTContext.isNull()) {

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7276,7 +7276,7 @@ void GenericSignatureBuilder::verifyGenericSignature(ASTContext &context,
72767276
}
72777277

72787278
// Canonicalize the signature to check that it is canonical.
7279-
(void)newSig->getCanonicalSignature();
7279+
(void)newSig.getCanonicalSignature();
72807280
}
72817281
}
72827282

@@ -7298,7 +7298,7 @@ void GenericSignatureBuilder::verifyGenericSignaturesInModule(
72987298
for (auto genericSig : allGenericSignatures) {
72997299
// Check whether this is the first time we've checked this (canonical)
73007300
// signature.
7301-
auto canGenericSig = genericSig->getCanonicalSignature();
7301+
auto canGenericSig = genericSig.getCanonicalSignature();
73027302
if (!knownGenericSignatures.insert(canGenericSig).second) continue;
73037303

73047304
verifyGenericSignature(context, canGenericSig);
@@ -7367,7 +7367,7 @@ AbstractGenericSignatureRequest::evaluate(
73677367
// generic signature builder.
73687368
if (!isCanonicalRequest(baseSignature, addedParameters, addedRequirements)) {
73697369
// Canonicalize the inputs so we can form the canonical request.
7370-
GenericSignature canBaseSignature = GenericSignature();
7370+
GenericSignature canBaseSignature;
73717371
if (baseSignature)
73727372
canBaseSignature = baseSignature->getCanonicalSignature();
73737373

‎lib/AST/ProtocolConformance.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,8 +547,8 @@ void NormalProtocolConformance::differenceAndStoreConditionalRequirements()
547547
}
548548

549549
auto extensionSig = ext->getGenericSignature();
550-
auto canExtensionSig = extensionSig->getCanonicalSignature();
551-
auto canTypeSig = typeSig->getCanonicalSignature();
550+
auto canExtensionSig = extensionSig.getCanonicalSignature();
551+
auto canTypeSig = typeSig.getCanonicalSignature();
552552
if (canTypeSig == canExtensionSig) {
553553
success({});
554554
return;

‎lib/AST/RequirementEnvironment.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ RequirementEnvironment::RequirementEnvironment(
140140
reqSig->getRequirements().size() == 1) {
141141
syntheticSignature = conformanceDC->getGenericSignatureOfContext();
142142
if (syntheticSignature) {
143-
syntheticSignature = syntheticSignature->getCanonicalSignature();
143+
syntheticSignature = syntheticSignature.getCanonicalSignature();
144144
syntheticEnvironment =
145145
syntheticSignature->getGenericEnvironment();
146146
}

‎lib/AST/SubstitutionMap.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ bool SubstitutionMap::isCanonical() const {
152152
SubstitutionMap SubstitutionMap::getCanonical() const {
153153
if (empty()) return *this;
154154

155-
auto canonicalSig = getGenericSignature()->getCanonicalSignature();
155+
auto canonicalSig = getGenericSignature().getCanonicalSignature();
156156
SmallVector<Type, 4> replacementTypes;
157157
for (Type replacementType : getReplacementTypesBuffer()) {
158158
if (replacementType)

‎lib/AST/Type.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,7 +1193,7 @@ CanType TypeBase::computeCanonicalType() {
11931193

11941194
CanGenericSignature genericSig;
11951195
if (auto *genericFnTy = dyn_cast<GenericFunctionType>(this))
1196-
genericSig = genericFnTy->getGenericSignature()->getCanonicalSignature();
1196+
genericSig = genericFnTy->getGenericSignature().getCanonicalSignature();
11971197

11981198
// Transform the parameter and result types.
11991199
SmallVector<AnyFunctionType::Param, 8> canParams;
@@ -2570,8 +2570,8 @@ static bool matches(CanType t1, CanType t2, TypeMatchOptions matchMode,
25702570
if (matchMode.contains(TypeMatchFlags::AllowCompatibleOpaqueTypeArchetypes))
25712571
if (auto opaque1 = t1->getAs<OpaqueTypeArchetypeType>())
25722572
if (auto opaque2 = t2->getAs<OpaqueTypeArchetypeType>())
2573-
return opaque1->getBoundSignature()->getCanonicalSignature() ==
2574-
opaque2->getBoundSignature()->getCanonicalSignature() &&
2573+
return opaque1->getBoundSignature().getCanonicalSignature() ==
2574+
opaque2->getBoundSignature().getCanonicalSignature() &&
25752575
opaque1->getInterfaceType()->getCanonicalType()->matches(
25762576
opaque2->getInterfaceType()->getCanonicalType(), matchMode);
25772577

@@ -3526,7 +3526,7 @@ static Type substGenericFunctionType(GenericFunctionType *genericFnType,
35263526
requirements.push_back(*substReqt);
35273527
}
35283528

3529-
GenericSignature genericSig = GenericSignature();
3529+
GenericSignature genericSig;
35303530
if (anySemanticChanges) {
35313531
// If there were semantic changes, we need to build a new generic
35323532
// signature.

‎lib/ClangImporter/ImportType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ namespace {
608608
importObjCTypeParamDecl(const clang::ObjCTypeParamDecl *objcTypeParamDecl) {
609609
// Pull the corresponding generic type parameter from the imported class.
610610
const auto *typeParamContext = objcTypeParamDecl->getDeclContext();
611-
GenericSignature genericSig = GenericSignature();
611+
GenericSignature genericSig;
612612
if (auto *category =
613613
dyn_cast<clang::ObjCCategoryDecl>(typeParamContext)) {
614614
auto ext = cast_or_null<ExtensionDecl>(

‎lib/IRGen/GenArchetype.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ llvm::Value *irgen::emitArchetypeWitnessTableRef(IRGenFunction &IGF,
189189
// concretely available; we really ought to be comparing the full paths
190190
// to this conformance from concrete sources.
191191

192-
auto signature = environment->getGenericSignature()->getCanonicalSignature();
192+
auto signature = environment->getGenericSignature().getCanonicalSignature();
193193
auto archetypeDepType = archetype->getInterfaceType();
194194

195195
auto astPath = signature->getConformanceAccessPath(archetypeDepType,
@@ -416,7 +416,7 @@ withOpaqueTypeGenericArgs(IRGenFunction &IGF,
416416
SmallVector<llvm::Type *, 4> types;
417417

418418
enumerateGenericSignatureRequirements(
419-
opaqueDecl->getGenericSignature()->getCanonicalSignature(),
419+
opaqueDecl->getGenericSignature().getCanonicalSignature(),
420420
[&](GenericRequirement reqt) {
421421
auto ty = reqt.TypeParameter.subst(archetype->getSubstitutions())
422422
->getCanonicalType(opaqueDecl->getGenericSignature());

‎lib/IRGen/GenHeap.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1460,7 +1460,7 @@ class FixedBoxTypeInfoBase : public BoxTypeInfo {
14601460

14611461
auto boxDescriptor = IGF.IGM.getAddrOfBoxDescriptor(
14621462
boxedInterfaceType,
1463-
env ? env->getGenericSignature()->getCanonicalSignature()
1463+
env ? env->getGenericSignature().getCanonicalSignature()
14641464
: CanGenericSignature());
14651465
llvm::Value *allocation = IGF.emitUnmanagedAlloc(layout, name,
14661466
boxDescriptor);

‎lib/IRGen/GenKeyPath.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ emitKeyPathComponent(IRGenModule &IGM,
699699
baseTy->getWithoutSpecifierType());
700700
// TODO: Eliminate GenericContextScope entirely
701701
GenericContextScope scope(IGM,
702-
genericEnv ? genericEnv->getGenericSignature()->getCanonicalSignature()
702+
genericEnv ? genericEnv->getGenericSignature().getCanonicalSignature()
703703
: nullptr);
704704
switch (auto kind = component.getKind()) {
705705
case KeyPathPatternComponent::Kind::StoredProperty: {
@@ -839,13 +839,11 @@ emitKeyPathComponent(IRGenModule &IGM,
839839
auto componentSig = externalDecl->getInnermostDeclContext()
840840
->getGenericSignatureOfContext();
841841

842-
auto componentCanSig = componentSig
843-
? componentSig->getCanonicalSignature()
844-
: CanGenericSignature();
842+
auto componentCanSig = componentSig.getCanonicalSignature();
845843
auto subs = component.getExternalSubstitutions();
846844
if (!subs.empty()) {
847845
enumerateGenericSignatureRequirements(
848-
componentSig->getCanonicalSignature(),
846+
componentCanSig,
849847
[&](GenericRequirement reqt) {
850848
auto substType = reqt.TypeParameter.subst(subs)
851849
->getCanonicalType();
@@ -1335,7 +1333,7 @@ void IRGenModule::emitSILProperty(SILProperty *prop) {
13351333
if (genericEnv) {
13361334
genericSig = prop->getDecl()->getInnermostDeclContext()
13371335
->getGenericSignatureOfContext()
1338-
->getCanonicalSignature();
1336+
.getCanonicalSignature();
13391337
enumerateGenericSignatureRequirements(genericSig,
13401338
[&](GenericRequirement reqt) { requirements.push_back(reqt); });
13411339
}

‎lib/IRGen/GenMeta.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ namespace {
325325
void addGenericParameters() {
326326
GenericSignature sig = asImpl().getGenericSignature();
327327
assert(sig);
328-
auto canSig = sig->getCanonicalSignature();
328+
auto canSig = sig.getCanonicalSignature();
329329

330330
canSig->forEachParam([&](GenericTypeParamType *param, bool canonical) {
331331
// Currently, there are only type parameters. The parameter is a key
@@ -1687,9 +1687,7 @@ namespace {
16871687
.subst(*O->getUnderlyingTypeSubstitutions())
16881688
->getCanonicalType(sig);
16891689

1690-
auto contextSig = O->getGenericSignature()
1691-
? O->getGenericSignature()->getCanonicalSignature()
1692-
: CanGenericSignature();
1690+
auto contextSig = O->getGenericSignature().getCanonicalSignature();
16931691

16941692
B.addRelativeAddress(IGM.getTypeRef(underlyingType, contextSig,
16951693
MangledTypeRefRole::Metadata).first);

‎lib/IRGen/GenProto.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2986,7 +2986,7 @@ GenericTypeRequirements::GenericTypeRequirements(IRGenModule &IGM,
29862986
if (!ncGenerics || ncGenerics->areAllParamsConcrete()) return;
29872987

29882988
// Construct a representative function type.
2989-
auto generics = ncGenerics->getCanonicalSignature();
2989+
auto generics = ncGenerics.getCanonicalSignature();
29902990
auto fnType = SILFunctionType::get(generics, SILFunctionType::ExtInfo(),
29912991
SILCoroutineKind::None,
29922992
/*callee*/ ParameterConvention::Direct_Unowned,
@@ -3030,7 +3030,7 @@ void GenericTypeRequirements::emitInitOfBuffer(IRGenFunction &IGF,
30303030
if (Requirements.empty()) return;
30313031

30323032
auto generics =
3033-
TheDecl->getGenericSignatureOfContext()->getCanonicalSignature();
3033+
TheDecl->getGenericSignatureOfContext().getCanonicalSignature();
30343034
auto &module = *TheDecl->getParentModule();
30353035
emitInitOfGenericRequirementsBuffer(IGF, Requirements, buffer,
30363036
[&](GenericRequirement requirement) {

0 commit comments

Comments
 (0)