Skip to content

Commit 1486d6b

Browse files
dan-zhengdan-zheng
authored
NFC: Add GenericSignature::getCanonicalSignature. (#29105)
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 0d87a14 commit 1486d6b

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

+184
-189
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: 6 additions & 6 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,9 +3277,9 @@ SILFunctionType::SILFunctionType(
32773277
"types and substitutions");
32783278

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

32853285
if (genericSig) {

‎lib/AST/ASTDemangler.cpp

Lines changed: 9 additions & 9 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 =
@@ -865,12 +865,13 @@ CanGenericSignature ASTBuilder::demangleGenericSignature(
865865
}
866866
}
867867

868-
return evaluateOrDefault(
869-
Ctx.evaluator,
870-
AbstractGenericSignatureRequest{
871-
nominalDecl->getGenericSignature().getPointer(), { },
872-
std::move(requirements)},
873-
GenericSignature())->getCanonicalSignature();
868+
return evaluateOrDefault(Ctx.evaluator,
869+
AbstractGenericSignatureRequest{
870+
nominalDecl->getGenericSignature().getPointer(),
871+
{},
872+
std::move(requirements)},
873+
GenericSignature())
874+
.getCanonicalSignature();
874875
}
875876

876877
DeclContext *
@@ -980,8 +981,7 @@ ASTBuilder::findDeclContext(NodePointer node) {
980981
continue;
981982
}
982983

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

‎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: 16 additions & 15 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,
@@ -414,21 +414,22 @@ withOpaqueTypeGenericArgs(IRGenFunction &IGF,
414414
} else {
415415
SmallVector<llvm::Value *, 4> args;
416416
SmallVector<llvm::Type *, 4> types;
417-
417+
418418
enumerateGenericSignatureRequirements(
419-
opaqueDecl->getGenericSignature()->getCanonicalSignature(),
420-
[&](GenericRequirement reqt) {
421-
auto ty = reqt.TypeParameter.subst(archetype->getSubstitutions())
422-
->getCanonicalType(opaqueDecl->getGenericSignature());
423-
if (reqt.Protocol) {
424-
auto ref = ProtocolConformanceRef(reqt.Protocol)
425-
.subst(reqt.TypeParameter, archetype->getSubstitutions());
426-
args.push_back(emitWitnessTableRef(IGF, ty, ref));
427-
} else {
428-
args.push_back(IGF.emitAbstractTypeMetadataRef(ty));
429-
}
430-
types.push_back(args.back()->getType());
431-
});
419+
opaqueDecl->getGenericSignature().getCanonicalSignature(),
420+
[&](GenericRequirement reqt) {
421+
auto ty = reqt.TypeParameter.subst(archetype->getSubstitutions())
422+
->getCanonicalType(opaqueDecl->getGenericSignature());
423+
if (reqt.Protocol) {
424+
auto ref =
425+
ProtocolConformanceRef(reqt.Protocol)
426+
.subst(reqt.TypeParameter, archetype->getSubstitutions());
427+
args.push_back(emitWitnessTableRef(IGF, ty, ref));
428+
} else {
429+
args.push_back(IGF.emitAbstractTypeMetadataRef(ty));
430+
}
431+
types.push_back(args.back()->getType());
432+
});
432433
auto bufTy = llvm::StructType::get(IGF.IGM.LLVMContext, types);
433434
alloca = IGF.createAlloca(bufTy, IGF.IGM.getPointerAlignment());
434435
allocaSize = IGF.IGM.getPointerSize() * args.size();

‎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);

0 commit comments

Comments
 (0)