Merge pull request #42063 from aschwaighofer/fix_internal_resilient_super_class_with_enable_testing

IRGen: Fix enable-testing of internal with resilient super class

Author: aschwaighofer

lib/IRGen/ClassMetadataVisitor.h

@@ -93,7 +93,7 @@ template <class Impl> class ClassMetadataVisitor
     asImpl().addIVarDestroyer();
 
     // Class members.
-    addClassMembers(Target);
+    addClassMembers(Target, Target);
   }
 
   /// Notes the beginning of the field offset vector for a particular ancestor
@@ -110,9 +110,11 @@ template <class Impl> class ClassMetadataVisitor
 
 private:
   /// Add fields associated with the given class and its bases.
-  void addClassMembers(ClassDecl *theClass) {
+  void addClassMembers(ClassDecl *theClass,
+                       ClassDecl *rootClass) {
     // Visit the superclass first.
     if (auto *superclassDecl = theClass->getSuperclassDecl()) {
+
       if (superclassDecl->hasClangNode()) {
         // Nothing to do; Objective-C classes do not add new members to
         // Swift class metadata.
@@ -123,10 +125,9 @@ template <class Impl> class ClassMetadataVisitor
       //    not publically accessible (e.g private or internal). This would
       //    normally not happen except if we compile theClass's module with
       //    enable-testing.
-      } else if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal) &&
-                 (theClass->getModuleContext() == IGM.getSwiftModule() ||
-                 theClass->getFormalAccessScope(/*useDC=*/nullptr,
-                            /*treatUsableFromInlineAsPublic=*/true).isPublic())) {
+      } else if (IGM.hasResilientMetadata(superclassDecl,
+                                          ResilienceExpansion::Maximal,
+                                          rootClass)) {
         // Runtime metadata instantiation will initialize our field offset
         // vector and vtable entries.
         //
@@ -137,7 +138,8 @@ template <class Impl> class ClassMetadataVisitor
         // NB: We don't apply superclass substitutions to members because we want
         // consistent metadata layout between generic superclasses and concrete
         // subclasses.
-        addClassMembers(superclassDecl);
+        addClassMembers(superclassDecl,
+                        rootClass);
       }
     }
 
@@ -151,7 +153,8 @@ template <class Impl> class ClassMetadataVisitor
 
     // If the class has resilient storage, we cannot make any assumptions about
     // its storage layout, so skip the rest of this method.
-    if (IGM.isResilient(theClass, ResilienceExpansion::Maximal))
+    if (IGM.isResilient(theClass, ResilienceExpansion::Maximal,
+                        rootClass))
       return;
 
     // A class only really *needs* a field-offset vector in the
@@ -175,7 +178,8 @@ template <class Impl> class ClassMetadataVisitor
 
     // If the class has resilient metadata, we cannot make any assumptions
     // about its metadata layout, so skip the rest of this method.
-    if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
+    if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal,
+                                 rootClass))
       return;
 
     // Add vtable entries.

lib/IRGen/GenClass.cpp

@@ -229,11 +229,14 @@ namespace {
         auto superclassDecl = superclassType.getClassOrBoundGenericClass();
         assert(superclassType && superclassDecl);
 
-        if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal))
+        if (IGM.hasResilientMetadata(superclassDecl,
+                                     ResilienceExpansion::Maximal,
+                                     rootClass))
           Options |= ClassMetadataFlags::ClassHasResilientAncestry;
 
         // If the superclass has resilient storage, don't walk its fields.
-        if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
+        if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal,
+                            rootClass)) {
           Options |= ClassMetadataFlags::ClassHasResilientMembers;
 
           // If the superclass is generic, we have to assume that its layout
@@ -263,10 +266,11 @@ namespace {
       if (classHasIncompleteLayout(IGM, theClass))
         Options |= ClassMetadataFlags::ClassHasMissingMembers;
 
-      if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
+      if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal,
+                                   rootClass))
         Options |= ClassMetadataFlags::ClassHasResilientAncestry;
 
-      if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
+      if (IGM.isResilient(theClass, ResilienceExpansion::Maximal, rootClass)) {
         Options |= ClassMetadataFlags::ClassHasResilientMembers;
         return;
       }

lib/IRGen/GenDecl.cpp

@@ -5334,6 +5334,22 @@ llvm::Constant *IRGenModule::getAddrOfGlobalUTF16String(StringRef utf8) {
   return address;
 }
 
+/// Can not treat a treat the layout of a class as resilient if the current
+///    class is defined in an external module and
+///    not publically accessible (e.g private or internal).
+/// This would normally not happen except if we compile theClass's module with
+/// enable-testing.
+static bool shouldTreatClassAsFragileBecauseOfEnableTesting(ClassDecl *D,
+                                                            IRGenModule &IGM) {
+    if (!D)
+      return false;
+
+    return D->getModuleContext() != IGM.getSwiftModule() &&
+           !D->getFormalAccessScope(/*useDC=*/nullptr,
+                                  /*treatUsableFromInlineAsPublic=*/true)
+           .isPublic();
+}
+
 /// Do we have to use resilient access patterns when working with this
 /// declaration?
 ///
@@ -5343,13 +5359,21 @@ llvm::Constant *IRGenModule::getAddrOfGlobalUTF16String(StringRef utf8) {
 /// - For classes, the superclass might change the size or number
 ///   of stored properties
 bool IRGenModule::isResilient(NominalTypeDecl *D,
-                              ResilienceExpansion expansion) {
+                              ResilienceExpansion expansion,
+                              ClassDecl *asViewedFromRootClass) {
+  assert(!asViewedFromRootClass || isa<ClassDecl>(D));
+
   if (D->getModuleContext()->getBypassResilience())
     return false;
   if (expansion == ResilienceExpansion::Maximal &&
       Types.getLoweringMode() == TypeConverter::Mode::CompletelyFragile) {
     return false;
   }
+
+  if (shouldTreatClassAsFragileBecauseOfEnableTesting(asViewedFromRootClass,
+                                                      *this))
+    return false;
+
   return D->isResilient(getSwiftModule(), expansion);
 }
 
@@ -5359,11 +5383,17 @@ bool IRGenModule::isResilient(NominalTypeDecl *D,
 /// For classes, this means that virtual method calls use dispatch thunks
 /// rather than accessing metadata members directly.
 bool IRGenModule::hasResilientMetadata(ClassDecl *D,
-                                       ResilienceExpansion expansion) {
+                                       ResilienceExpansion expansion,
+                                       ClassDecl *asViewedFromRootClass) {
   if (expansion == ResilienceExpansion::Maximal &&
       Types.getLoweringMode() == TypeConverter::Mode::CompletelyFragile) {
     return false;
   }
+
+  if (shouldTreatClassAsFragileBecauseOfEnableTesting(asViewedFromRootClass,
+                                                      *this))
+    return false;
+
   return D->hasResilientMetadata(getSwiftModule(), expansion);
 }
 

lib/IRGen/IRGenModule.h

@@ -977,8 +977,10 @@ class IRGenModule {
   substOpaqueTypesWithUnderlyingTypes(CanType type,
                                       ProtocolConformanceRef conformance);
 
-  bool isResilient(NominalTypeDecl *decl, ResilienceExpansion expansion);
-  bool hasResilientMetadata(ClassDecl *decl, ResilienceExpansion expansion);
+  bool isResilient(NominalTypeDecl *decl, ResilienceExpansion expansion,
+                   ClassDecl *asViewedFromRootClass = nullptr);
+  bool hasResilientMetadata(ClassDecl *decl, ResilienceExpansion expansion,
+                            ClassDecl *asViewedFromRootClass = nullptr);
   ResilienceExpansion getResilienceExpansionForAccess(NominalTypeDecl *decl);
   ResilienceExpansion getResilienceExpansionForLayout(NominalTypeDecl *decl);
   ResilienceExpansion getResilienceExpansionForLayout(SILGlobalVariable *var);

test/IRGen/Inputs/resilient-class.swift

@@ -1,7 +1,18 @@
 open class Base {
-    var x = 1
+   var x = 1
+}
+internal struct TypeContainer {
+
+  enum SomeEnum:String {
+    case FirstCase = "first"
+    case SecondCase = "second"
+  }
 }
 
 internal class SubClass : Base {
-    var y = 2
+    var y : TypeContainer.SomeEnum
+
+    override init() {
+        y = .FirstCase
+    }
 }

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

@@ -7,14 +7,22 @@
 // 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
 // RUN: %target-build-swift -I %t -L %t -lresilient %s -o %t/main %target-rpath(%t)
 // RUN: %target-build-swift -O -I %t -L %t -lresilient %s -o %t/main %target-rpath(%t)
+// RUN: %target-run %t/main %t/%target-library-name(resilient) | %FileCheck --check-prefix=EXEC-CHECK %s
 
 @testable import resilient
 
 // Don't access via the class offset global. Use a fragile access pattern instead.
 
 // CHECK-NOT: s9resilient8SubClassCMo
 
+public func isEqual<T:Equatable>(_ l: T, _ r: T) -> Bool {
+    return l == r
+}
+
 public func testCase() {
   let t = SubClass()
-  print(t.y)
+  print(t.y)  // EXEC-CHECK: FirstCase
+  print("isEqual \(isEqual(t.y, .FirstCase))") // EXEC-CHECK: isEqual true
 }
+
+testCase()