Skip to content

Commit 3930fc4

Browse files
committed
IRGen: Fix enable-testing of internal with resilient super class
Enable testing makes `internal` types visible from outside the module. We can no longer treat super classes as resilient. Follow-up to #41044. rdar://90489618
1 parent 5f99c31 commit 3930fc4

File tree

6 files changed

+79
-20
lines changed

6 files changed

+79
-20
lines changed

lib/IRGen/ClassMetadataVisitor.h

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ template <class Impl> class ClassMetadataVisitor
9393
asImpl().addIVarDestroyer();
9494

9595
// Class members.
96-
addClassMembers(Target);
96+
addClassMembers(Target, Target);
9797
}
9898

9999
/// Notes the beginning of the field offset vector for a particular ancestor
@@ -110,9 +110,11 @@ template <class Impl> class ClassMetadataVisitor
110110

111111
private:
112112
/// Add fields associated with the given class and its bases.
113-
void addClassMembers(ClassDecl *theClass) {
113+
void addClassMembers(ClassDecl *theClass,
114+
ClassDecl *rootClass) {
114115
// Visit the superclass first.
115116
if (auto *superclassDecl = theClass->getSuperclassDecl()) {
117+
116118
if (superclassDecl->hasClangNode()) {
117119
// Nothing to do; Objective-C classes do not add new members to
118120
// Swift class metadata.
@@ -123,10 +125,9 @@ template <class Impl> class ClassMetadataVisitor
123125
// not publically accessible (e.g private or internal). This would
124126
// normally not happen except if we compile theClass's module with
125127
// enable-testing.
126-
} else if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal) &&
127-
(theClass->getModuleContext() == IGM.getSwiftModule() ||
128-
theClass->getFormalAccessScope(/*useDC=*/nullptr,
129-
/*treatUsableFromInlineAsPublic=*/true).isPublic())) {
128+
} else if (IGM.hasResilientMetadata(superclassDecl,
129+
ResilienceExpansion::Maximal,
130+
rootClass)) {
130131
// Runtime metadata instantiation will initialize our field offset
131132
// vector and vtable entries.
132133
//
@@ -137,7 +138,8 @@ template <class Impl> class ClassMetadataVisitor
137138
// NB: We don't apply superclass substitutions to members because we want
138139
// consistent metadata layout between generic superclasses and concrete
139140
// subclasses.
140-
addClassMembers(superclassDecl);
141+
addClassMembers(superclassDecl,
142+
rootClass);
141143
}
142144
}
143145

@@ -151,7 +153,8 @@ template <class Impl> class ClassMetadataVisitor
151153

152154
// If the class has resilient storage, we cannot make any assumptions about
153155
// its storage layout, so skip the rest of this method.
154-
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal))
156+
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal,
157+
rootClass))
155158
return;
156159

157160
// A class only really *needs* a field-offset vector in the
@@ -175,7 +178,8 @@ template <class Impl> class ClassMetadataVisitor
175178

176179
// If the class has resilient metadata, we cannot make any assumptions
177180
// about its metadata layout, so skip the rest of this method.
178-
if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
181+
if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal,
182+
rootClass))
179183
return;
180184

181185
// Add vtable entries.

lib/IRGen/GenClass.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,14 @@ namespace {
229229
auto superclassDecl = superclassType.getClassOrBoundGenericClass();
230230
assert(superclassType && superclassDecl);
231231

232-
if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal))
232+
if (IGM.hasResilientMetadata(superclassDecl,
233+
ResilienceExpansion::Maximal,
234+
rootClass))
233235
Options |= ClassMetadataFlags::ClassHasResilientAncestry;
234236

235237
// If the superclass has resilient storage, don't walk its fields.
236-
if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
238+
if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal,
239+
rootClass)) {
237240
Options |= ClassMetadataFlags::ClassHasResilientMembers;
238241

239242
// If the superclass is generic, we have to assume that its layout
@@ -263,10 +266,11 @@ namespace {
263266
if (classHasIncompleteLayout(IGM, theClass))
264267
Options |= ClassMetadataFlags::ClassHasMissingMembers;
265268

266-
if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
269+
if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal,
270+
rootClass))
267271
Options |= ClassMetadataFlags::ClassHasResilientAncestry;
268272

269-
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
273+
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal, rootClass)) {
270274
Options |= ClassMetadataFlags::ClassHasResilientMembers;
271275
return;
272276
}

lib/IRGen/GenDecl.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5334,6 +5334,22 @@ llvm::Constant *IRGenModule::getAddrOfGlobalUTF16String(StringRef utf8) {
53345334
return address;
53355335
}
53365336

5337+
/// Can not treat a treat the layout of a class as resilient if the current
5338+
/// class is defined in an external module and
5339+
/// not publically accessible (e.g private or internal).
5340+
/// This would normally not happen except if we compile theClass's module with
5341+
/// enable-testing.
5342+
static bool shouldTreatClassAsFragileBecauseOfEnableTesting(ClassDecl *D,
5343+
IRGenModule &IGM) {
5344+
if (!D)
5345+
return false;
5346+
5347+
return D->getModuleContext() != IGM.getSwiftModule() &&
5348+
!D->getFormalAccessScope(/*useDC=*/nullptr,
5349+
/*treatUsableFromInlineAsPublic=*/true)
5350+
.isPublic();
5351+
}
5352+
53375353
/// Do we have to use resilient access patterns when working with this
53385354
/// declaration?
53395355
///
@@ -5343,13 +5359,21 @@ llvm::Constant *IRGenModule::getAddrOfGlobalUTF16String(StringRef utf8) {
53435359
/// - For classes, the superclass might change the size or number
53445360
/// of stored properties
53455361
bool IRGenModule::isResilient(NominalTypeDecl *D,
5346-
ResilienceExpansion expansion) {
5362+
ResilienceExpansion expansion,
5363+
ClassDecl *asViewedFromRootClass) {
5364+
assert(!asViewedFromRootClass || isa<ClassDecl>(D));
5365+
53475366
if (D->getModuleContext()->getBypassResilience())
53485367
return false;
53495368
if (expansion == ResilienceExpansion::Maximal &&
53505369
Types.getLoweringMode() == TypeConverter::Mode::CompletelyFragile) {
53515370
return false;
53525371
}
5372+
5373+
if (shouldTreatClassAsFragileBecauseOfEnableTesting(asViewedFromRootClass,
5374+
*this))
5375+
return false;
5376+
53535377
return D->isResilient(getSwiftModule(), expansion);
53545378
}
53555379

@@ -5359,11 +5383,17 @@ bool IRGenModule::isResilient(NominalTypeDecl *D,
53595383
/// For classes, this means that virtual method calls use dispatch thunks
53605384
/// rather than accessing metadata members directly.
53615385
bool IRGenModule::hasResilientMetadata(ClassDecl *D,
5362-
ResilienceExpansion expansion) {
5386+
ResilienceExpansion expansion,
5387+
ClassDecl *asViewedFromRootClass) {
53635388
if (expansion == ResilienceExpansion::Maximal &&
53645389
Types.getLoweringMode() == TypeConverter::Mode::CompletelyFragile) {
53655390
return false;
53665391
}
5392+
5393+
if (shouldTreatClassAsFragileBecauseOfEnableTesting(asViewedFromRootClass,
5394+
*this))
5395+
return false;
5396+
53675397
return D->hasResilientMetadata(getSwiftModule(), expansion);
53685398
}
53695399

lib/IRGen/IRGenModule.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -977,8 +977,10 @@ class IRGenModule {
977977
substOpaqueTypesWithUnderlyingTypes(CanType type,
978978
ProtocolConformanceRef conformance);
979979

980-
bool isResilient(NominalTypeDecl *decl, ResilienceExpansion expansion);
981-
bool hasResilientMetadata(ClassDecl *decl, ResilienceExpansion expansion);
980+
bool isResilient(NominalTypeDecl *decl, ResilienceExpansion expansion,
981+
ClassDecl *asViewedFromRootClass = nullptr);
982+
bool hasResilientMetadata(ClassDecl *decl, ResilienceExpansion expansion,
983+
ClassDecl *asViewedFromRootClass = nullptr);
982984
ResilienceExpansion getResilienceExpansionForAccess(NominalTypeDecl *decl);
983985
ResilienceExpansion getResilienceExpansionForLayout(NominalTypeDecl *decl);
984986
ResilienceExpansion getResilienceExpansionForLayout(SILGlobalVariable *var);
Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
open class Base {
2-
var x = 1
2+
var x = 1
3+
}
4+
internal struct TypeContainer {
5+
6+
enum SomeEnum:String {
7+
case FirstCase = "first"
8+
case SecondCase = "second"
9+
}
310
}
411

512
internal class SubClass : Base {
6-
var y = 2
13+
var y : TypeContainer.SomeEnum
14+
15+
override init() {
16+
y = .FirstCase
17+
}
718
}

test/IRGen/testing-enabled-resilient-super-class.swift

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,22 @@
77
// RUN: %target-build-swift-dylib(%t/%target-library-name(resilient)) %S/Inputs/resilient-class.swift -module-name resilient -emit-module -emit-module-path %t/resilient.swiftmodule -enable-library-evolution -enable-testing
88
// RUN: %target-build-swift -I %t -L %t -lresilient %s -o %t/main %target-rpath(%t)
99
// RUN: %target-build-swift -O -I %t -L %t -lresilient %s -o %t/main %target-rpath(%t)
10+
// RUN: %target-run %t/main %t/%target-library-name(resilient) | %FileCheck --check-prefix=EXEC-CHECK %s
1011

1112
@testable import resilient
1213

1314
// Don't access via the class offset global. Use a fragile access pattern instead.
1415

1516
// CHECK-NOT: s9resilient8SubClassCMo
1617

18+
public func isEqual<T:Equatable>(_ l: T, _ r: T) -> Bool {
19+
return l == r
20+
}
21+
1722
public func testCase() {
1823
let t = SubClass()
19-
print(t.y)
24+
print(t.y) // EXEC-CHECK: FirstCase
25+
print("isEqual \(isEqual(t.y, .FirstCase))") // EXEC-CHECK: isEqual true
2026
}
27+
28+
testCase()

0 commit comments

Comments
 (0)