Skip to content

Commit 2cfc2fa

Browse files
slavapestovslavapestov
authored
Merge pull request swiftlang#30054 from slavapestov/vtable-thunk-generic-subclass-non-generic-base-class
Fix vtable thunk emission with a generic subclass of a non-generic base class
2 parents 482c82c + 2a865a0 commit 2cfc2fa

File tree

3 files changed

+71
-56
lines changed

3 files changed

+71
-56
lines changed

‎lib/AST/ASTContext.cpp

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,14 @@ using AssociativityCacheType =
102102
struct OverrideSignatureKey {
103103
GenericSignature baseMethodSig;
104104
GenericSignature derivedMethodSig;
105-
Type superclassTy, subclassTy;
105+
Decl *subclassDecl;
106106

107107
OverrideSignatureKey(GenericSignature baseMethodSignature,
108108
GenericSignature derivedMethodSignature,
109-
Type superclassType,
110-
Type subclassType)
109+
Decl *subclassDecl)
111110
: baseMethodSig(baseMethodSignature),
112111
derivedMethodSig(derivedMethodSignature),
113-
superclassTy(superclassType),
114-
subclassTy(subclassType) {}
112+
subclassDecl(subclassDecl) {}
115113
};
116114

117115
namespace llvm {
@@ -123,31 +121,27 @@ template <> struct DenseMapInfo<OverrideSignatureKey> {
123121
const OverrideSignatureKey rhs) {
124122
return lhs.baseMethodSig.getPointer() == rhs.baseMethodSig.getPointer() &&
125123
lhs.derivedMethodSig.getPointer() == rhs.derivedMethodSig.getPointer() &&
126-
lhs.superclassTy.getPointer() == rhs.superclassTy.getPointer() &&
127-
lhs.subclassTy.getPointer() == rhs.subclassTy.getPointer();
124+
lhs.subclassDecl == rhs.subclassDecl;
128125
}
129126

130127
static inline OverrideSignatureKey getEmptyKey() {
131128
return OverrideSignatureKey(DenseMapInfo<GenericSignature>::getEmptyKey(),
132129
DenseMapInfo<GenericSignature>::getEmptyKey(),
133-
DenseMapInfo<Type>::getEmptyKey(),
134-
DenseMapInfo<Type>::getEmptyKey());
130+
DenseMapInfo<Decl *>::getEmptyKey());
135131
}
136132

137133
static inline OverrideSignatureKey getTombstoneKey() {
138134
return OverrideSignatureKey(
139135
DenseMapInfo<GenericSignature>::getTombstoneKey(),
140136
DenseMapInfo<GenericSignature>::getTombstoneKey(),
141-
DenseMapInfo<Type>::getTombstoneKey(),
142-
DenseMapInfo<Type>::getTombstoneKey());
137+
DenseMapInfo<Decl *>::getTombstoneKey());
143138
}
144139

145140
static unsigned getHashValue(const OverrideSignatureKey &Val) {
146141
return hash_combine(
147142
DenseMapInfo<GenericSignature>::getHashValue(Val.baseMethodSig),
148143
DenseMapInfo<GenericSignature>::getHashValue(Val.derivedMethodSig),
149-
DenseMapInfo<Type>::getHashValue(Val.superclassTy),
150-
DenseMapInfo<Type>::getHashValue(Val.subclassTy));
144+
DenseMapInfo<Decl *>::getHashValue(Val.subclassDecl));
151145
}
152146
};
153147
} // namespace llvm
@@ -4471,40 +4465,39 @@ CanGenericSignature ASTContext::getOpenedArchetypeSignature(CanType existential,
44714465
GenericSignature
44724466
ASTContext::getOverrideGenericSignature(const ValueDecl *base,
44734467
const ValueDecl *derived) {
4474-
auto baseGenericCtx = base->getAsGenericContext();
4475-
auto derivedGenericCtx = derived->getAsGenericContext();
4468+
assert(isa<AbstractFunctionDecl>(base) || isa<SubscriptDecl>(base));
4469+
assert(isa<AbstractFunctionDecl>(derived) || isa<SubscriptDecl>(derived));
44764470

4477-
if (!baseGenericCtx || !derivedGenericCtx)
4478-
return nullptr;
4471+
auto baseClass = base->getDeclContext()->getSelfClassDecl();
4472+
auto derivedClass = derived->getDeclContext()->getSelfClassDecl();
44794473

4480-
if (base == derived)
4481-
return derivedGenericCtx->getGenericSignature();
4474+
assert(baseClass != nullptr);
4475+
assert(derivedClass != nullptr);
44824476

4483-
auto baseClass = base->getDeclContext()->getSelfClassDecl();
4484-
if (!baseClass)
4485-
return nullptr;
4477+
auto baseGenericSig = base->getAsGenericContext()->getGenericSignature();
4478+
auto derivedGenericSig = derived->getAsGenericContext()->getGenericSignature();
44864479

4487-
auto derivedClass = derived->getDeclContext()->getSelfClassDecl();
4488-
if (!derivedClass)
4489-
return nullptr;
4480+
if (base == derived)
4481+
return derivedGenericSig;
44904482

44914483
if (derivedClass->getSuperclass().isNull())
44924484
return nullptr;
44934485

4494-
if (baseGenericCtx->getGenericSignature().isNull() ||
4495-
derivedGenericCtx->getGenericSignature().isNull())
4486+
if (derivedGenericSig.isNull())
44964487
return nullptr;
44974488

4489+
if (baseGenericSig.isNull())
4490+
return derivedGenericSig;
4491+
44984492
auto baseClassSig = baseClass->getGenericSignature();
44994493
auto subMap = derivedClass->getSuperclass()->getContextSubstitutionMap(
45004494
derivedClass->getModuleContext(), baseClass);
45014495

45024496
unsigned derivedDepth = 0;
45034497

4504-
auto key = OverrideSignatureKey(baseGenericCtx->getGenericSignature(),
4505-
derivedGenericCtx->getGenericSignature(),
4506-
derivedClass->getSuperclass(),
4507-
derivedClass->getDeclaredInterfaceType());
4498+
auto key = OverrideSignatureKey(baseGenericSig,
4499+
derivedGenericSig,
4500+
derivedClass);
45084501

45094502
if (getImpl().overrideSigCache.find(key) !=
45104503
getImpl().overrideSigCache.end()) {
@@ -4515,7 +4508,7 @@ ASTContext::getOverrideGenericSignature(const ValueDecl *base,
45154508
derivedDepth = derivedSig->getGenericParams().back()->getDepth() + 1;
45164509

45174510
SmallVector<GenericTypeParamType *, 2> addedGenericParams;
4518-
if (auto *gpList = derivedGenericCtx->getGenericParams()) {
4511+
if (auto *gpList = derived->getAsGenericContext()->getGenericParams()) {
45194512
for (auto gp : *gpList) {
45204513
addedGenericParams.push_back(
45214514
gp->getDeclaredInterfaceType()->castTo<GenericTypeParamType>());
@@ -4549,7 +4542,7 @@ ASTContext::getOverrideGenericSignature(const ValueDecl *base,
45494542
};
45504543

45514544
SmallVector<Requirement, 2> addedRequirements;
4552-
for (auto reqt : baseGenericCtx->getGenericSignature()->getRequirements()) {
4545+
for (auto reqt : baseGenericSig->getRequirements()) {
45534546
if (auto substReqt = reqt.subst(substFn, lookupConformanceFn)) {
45544547
addedRequirements.push_back(*substReqt);
45554548
}

‎lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,17 @@ bool swift::isOverrideBasedOnType(const ValueDecl *decl, Type declTy,
163163
//
164164
// We can still succeed with a subtype match later in
165165
// OverrideMatcher::match().
166-
auto declGenericCtx = decl->getAsGenericContext();
167-
auto &ctx = decl->getASTContext();
168-
auto sig = ctx.getOverrideGenericSignature(parentDecl, decl);
169-
170-
if (sig && declGenericCtx &&
171-
declGenericCtx->getGenericSignature().getCanonicalSignature() !=
172-
sig.getCanonicalSignature()) {
173-
return false;
166+
if (decl->getDeclContext()->getSelfClassDecl()) {
167+
if (auto declGenericCtx = decl->getAsGenericContext()) {
168+
auto &ctx = decl->getASTContext();
169+
auto sig = ctx.getOverrideGenericSignature(parentDecl, decl);
170+
171+
if (sig &&
172+
declGenericCtx->getGenericSignature().getCanonicalSignature() !=
173+
sig.getCanonicalSignature()) {
174+
return false;
175+
}
176+
}
174177
}
175178

176179
// If this is a constructor, let's compare only parameter types.
@@ -1060,21 +1063,23 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
10601063
emittedMatchError = true;
10611064
}
10621065

1063-
auto baseGenericCtx = baseDecl->getAsGenericContext();
1064-
auto derivedGenericCtx = decl->getAsGenericContext();
1065-
1066-
using Direction = ASTContext::OverrideGenericSignatureReqCheck;
1067-
if (baseGenericCtx && derivedGenericCtx) {
1068-
if (!ctx.overrideGenericSignatureReqsSatisfied(
1069-
baseDecl, decl, Direction::DerivedReqSatisfiedByBase)) {
1070-
auto newSig = ctx.getOverrideGenericSignature(baseDecl, decl);
1071-
diags.diagnose(decl, diag::override_method_different_generic_sig,
1072-
decl->getBaseName(),
1073-
derivedGenericCtx->getGenericSignature()->getAsString(),
1074-
baseGenericCtx->getGenericSignature()->getAsString(),
1075-
newSig->getAsString());
1076-
diags.diagnose(baseDecl, diag::overridden_here);
1077-
emittedMatchError = true;
1066+
if (isClassOverride()) {
1067+
auto baseGenericCtx = baseDecl->getAsGenericContext();
1068+
auto derivedGenericCtx = decl->getAsGenericContext();
1069+
1070+
using Direction = ASTContext::OverrideGenericSignatureReqCheck;
1071+
if (baseGenericCtx && derivedGenericCtx) {
1072+
if (!ctx.overrideGenericSignatureReqsSatisfied(
1073+
baseDecl, decl, Direction::DerivedReqSatisfiedByBase)) {
1074+
auto newSig = ctx.getOverrideGenericSignature(baseDecl, decl);
1075+
diags.diagnose(decl, diag::override_method_different_generic_sig,
1076+
decl->getBaseName(),
1077+
derivedGenericCtx->getGenericSignature()->getAsString(),
1078+
baseGenericCtx->getGenericSignature()->getAsString(),
1079+
newSig->getAsString());
1080+
diags.diagnose(baseDecl, diag::overridden_here);
1081+
emittedMatchError = true;
1082+
}
10781083
}
10791084
}
10801085

‎test/SILGen/vtable_thunks.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,16 @@ class Noot : Aap {
228228
override func map() -> (S?) -> () -> Noot {}
229229
}
230230

231+
// rdar://problem/59669591
232+
class Base {
233+
func returnsOptional() -> Int? { return nil }
234+
}
235+
236+
class Derived<Foo> : Base {
237+
// The thunk here needs to be generic.
238+
override func returnsOptional() -> Int { return 0 }
239+
}
240+
231241
// CHECK-LABEL: sil private [thunk] [ossa] @$s13vtable_thunks3BarC3foo{{[_0-9a-zA-Z]*}}FTV : $@convention(method) (@guaranteed @callee_guaranteed (Int) -> Int, @guaranteed Bar) -> @owned Optional<@callee_guaranteed (Int) -> Int>
232242
// CHECK: [[IMPL:%.*]] = function_ref @$s13vtable_thunks3BarC3foo{{[_0-9a-zA-Z]*}}F
233243
// CHECK: apply [[IMPL]]
@@ -257,6 +267,8 @@ class Noot : Aap {
257267
// CHECK: [[OUTER:%.*]] = convert_function [[INNER]] : $@callee_guaranteed () -> @owned Noot to $@callee_guaranteed () -> @owned Optional<Aap>
258268
// CHECK: return [[OUTER]]
259269

270+
// CHECK-LABEL: sil private [thunk] [ossa] @$s13vtable_thunks7DerivedC15returnsOptionalSiyFAA4BaseCADSiSgyFTV : $@convention(method) <τ_0_0> (@guaranteed Derived<τ_0_0>) -> Optional<Int> {
271+
260272
// CHECK-LABEL: sil_vtable D {
261273
// CHECK: #B.iuo!1: {{.*}} : @$s13vtable_thunks1D{{[A-Z0-9a-z_]*}}FTV
262274
// CHECK: #B.f!1: {{.*}} : @$s13vtable_thunks1D{{[A-Z0-9a-z_]*}}F
@@ -321,3 +333,8 @@ class Noot : Aap {
321333
// CHECK-LABEL: sil_vtable NoThrowVariance {
322334
// CHECK: #ThrowVariance.mightThrow!1: {{.*}} : @$s13vtable_thunks{{[A-Z0-9a-z_]*}}F
323335

336+
// CHECK-LABEL: sil_vtable Base {
337+
// CHECK: #Base.returnsOptional!1: (Base) -> () -> Int? : @$s13vtable_thunks4BaseC15returnsOptionalSiSgyF
338+
339+
// CHECK-LABEL: sil_vtable Derived {
340+
// CHECK: #Base.returnsOptional!1: (Base) -> () -> Int? : @$s13vtable_thunks7DerivedC15returnsOptionalSiyFAA4BaseCADSiSgyFTV [override]

0 commit comments

Comments
 (0)