Merge pull request #18299 from jckarter/enable-key-path-resilience

Enable key path resilience.

Author: jckarter

include/swift/Basic/LangOptions.h

@@ -226,7 +226,7 @@ namespace swift {
     bool EnableSILOpaqueValues = false;
 
     /// Enables key path resilience.
-    bool EnableKeyPathResilience = false;
+    bool EnableKeyPathResilience = true;
 
     /// If set to true, the diagnosis engine can assume the emitted diagnostics
     /// will be used in editor. This usually leads to more aggressive fixit.

lib/IRGen/GenDecl.cpp

@@ -1582,11 +1582,18 @@ SILLinkage LinkEntity::getLinkage(ForDefinition_t forDefinition) const {
     return getSILLinkage(linkage, forDefinition);
   }
 
+  case Kind::PropertyDescriptor: {
+    // Return the linkage of the getter, which may be more permissive than the
+    // property itself (for instance, with a private/internal property whose
+    // accessor is @inlinable or @usableFromInline)
+    auto getterDecl = cast<AbstractStorageDecl>(getDecl())->getGetter();
+    return getSILLinkage(getDeclLinkage(getterDecl), forDefinition);
+  }
+
   case Kind::ObjCClass:
   case Kind::ObjCMetaclass:
   case Kind::SwiftMetaclassStub:
   case Kind::NominalTypeDescriptor:
-  case Kind::PropertyDescriptor:
   case Kind::ClassMetadataBaseOffset:
   case Kind::ProtocolDescriptor:
     return getSILLinkage(getDeclLinkage(getDecl()), forDefinition);

lib/IRGen/GenKeyPath.cpp

@@ -46,11 +46,16 @@
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/Types.h"
+#include "swift/Basic/Statistic.h"
 #include "swift/IRGen/Linking.h"
 
 using namespace swift;
 using namespace irgen;
 
+#define DEBUG_TYPE "IRGen key paths"
+STATISTIC(NumTrivialPropertyDescriptors, "# of trivial property descriptors");
+STATISTIC(NumNonTrivialPropertyDescriptors, "# of nontrivial property descriptors");
+
 enum KeyPathAccessor {
   Getter,
   Setter,
@@ -910,23 +915,43 @@ emitKeyPathComponent(IRGenModule &IGM,
         idValue = IGM.getAddrOfObjCSelectorRef(declRef);
         idResolved = false;
       } else {
-        idKind = KeyPathComponentHeader::VTableOffset;
+        if (auto overridden = declRef.getOverriddenVTableEntry())
+          declRef = overridden;
+
         auto dc = declRef.getDecl()->getDeclContext();
+
+        // If the method context is resilient, use the dispatch thunk as a
+        // stable identifier for the storage.
+        if (IGM.isResilient(cast<NominalTypeDecl>(dc),
+                            ResilienceExpansion::Minimal)) {
+          idKind = KeyPathComponentHeader::Pointer;
+          idValue = IGM.getAddrOfDispatchThunk(declRef, NotForDefinition);
+          idResolved = true;
+          break;
+        }
+      
+        idKind = KeyPathComponentHeader::VTableOffset;
         if (isa<ClassDecl>(dc) && !cast<ClassDecl>(dc)->isForeign()) {
-          auto overridden = declRef.getOverriddenVTableEntry();
           auto declaringClass =
-            cast<ClassDecl>(overridden.getDecl()->getDeclContext());
+            cast<ClassDecl>(declRef.getDecl()->getDeclContext());
           auto &metadataLayout = IGM.getClassMetadataLayout(declaringClass);
-          // FIXME: Resilience. We don't want vtable layout to be ABI, so this
-          // should be encoded as a reference to the method dispatch thunk
-          // instead.
-          auto offset = metadataLayout.getStaticMethodOffset(overridden);
+
+          // For a class method, we don't necessarily need the absolute offset,
+          // only an offset that's unique to this method. For a class with
+          // resilient ancestry, all of the superclass methods will be
+          // identified by their dispatch thunk (see above), so we can use
+          // relative offsets from the dynamic base offset to identify the local
+          // class's own methods.
+          auto methodInfo = metadataLayout.getMethodOffsetInfo(declRef);
+          Size offset;
+          if (methodInfo.isStatic())
+            offset = methodInfo.getStaticOffset();
+          else
+            offset = methodInfo.getRelativeOffset();
+
           idValue = llvm::ConstantInt::get(IGM.SizeTy, offset.getValue());
           idResolved = true;
         } else if (auto methodProto = dyn_cast<ProtocolDecl>(dc)) {
-          // FIXME: Resilience. We don't want witness table layout to be ABI,
-          // so this should be encoded as a reference to the method dispatch
-          // thunk instead.
           auto &protoInfo = IGM.getProtocolInfo(methodProto);
           auto index = protoInfo.getFunctionIndex(
                                cast<AbstractFunctionDecl>(declRef.getDecl()));
@@ -1187,6 +1212,7 @@ IRGenModule::getAddrOfKeyPathPattern(KeyPathPattern *pattern,
 
 void IRGenModule::emitSILProperty(SILProperty *prop) {
   if (prop->isTrivial()) {
+    ++NumTrivialPropertyDescriptors;
     // All trivial property descriptors can share a single definition in the
     // translation unit.
     if (!TheTrivialPropertyDescriptor) {
@@ -1213,6 +1239,8 @@ void IRGenModule::emitSILProperty(SILProperty *prop) {
     return;
   }
 
+  ++NumNonTrivialPropertyDescriptors;
+
   ConstantInitBuilder builder(*this);
   ConstantStructBuilder fields = builder.beginStruct();
   fields.setPacked(true);

lib/IRGen/MetadataLayout.cpp

@@ -400,12 +400,9 @@ ClassMetadataLayout::getMethodInfo(IRGenFunction &IGF, SILDeclRef method) const{
   return MethodInfo(offset);
 }
 
-Size ClassMetadataLayout::getStaticMethodOffset(SILDeclRef method) const{
-  auto &stored = getStoredMethodInfo(method);
-
-  assert(stored.TheOffset.isStatic() &&
-         "resilient class metadata layout unsupported!");
-  return stored.TheOffset.getStaticOffset();
+MetadataLayout::StoredOffset
+ClassMetadataLayout::getMethodOffsetInfo(SILDeclRef method) const {
+  return getStoredMethodInfo(method).TheOffset;
 }
 
 Offset

lib/IRGen/MetadataLayout.h

@@ -249,12 +249,9 @@ class ClassMetadataLayout : public NominalMetadataLayout {
 
   MethodInfo getMethodInfo(IRGenFunction &IGF, SILDeclRef method) const;
 
-  /// Assuming that the given method is at a static offset in the metadata,
-  /// return that static offset.
-  ///
-  /// DEPRECATED: callers should be updated to handle this in a
-  /// more arbitrary fashion.
-  Size getStaticMethodOffset(SILDeclRef method) const;
+  /// Return the information necessary to compute the offset of a method's
+  /// vtable entry in the class object.
+  StoredOffset getMethodOffsetInfo(SILDeclRef method) const;
 
   Offset getFieldOffset(IRGenFunction &IGF, VarDecl *field) const;
 

lib/SIL/SIL.cpp

@@ -19,7 +19,9 @@
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/AnyFunctionRef.h"
 #include "swift/AST/Decl.h"
+#include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/Pattern.h"
+#include "swift/AST/ParameterList.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/ClangImporter/ClangModule.h"
 #include "clang/AST/Attr.h"
@@ -218,6 +220,10 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const {
   if (!getDeclContext()->isTypeContext() || isStatic())
     return false;
 
+  // Protocol requirements do not need property descriptors.
+  if (isa<ProtocolDecl>(getDeclContext()))
+    return false;
+  
   // Any property that's potentially resilient should have accessors
   // synthesized.
   if (!getGetter())
@@ -230,6 +236,7 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const {
   // TODO: If previous versions of an ABI-stable binary needed the descriptor,
   // then we still do.
 
+  // Check the linkage of the declaration.
   auto getter = SILDeclRef(getGetter());
   auto getterLinkage = getter.getLinkage(ForDefinition);
 
@@ -252,5 +259,33 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const {
     llvm_unreachable("should be definition linkage?");
   }
 
+  // Subscripts with inout arguments (FIXME)and reabstracted arguments(/FIXME)
+  // don't have descriptors either.
+  if (auto sub = dyn_cast<SubscriptDecl>(this)) {
+    for (auto *index : *sub->getIndices()) {
+      // Keypaths can't capture inout indices.
+      if (index->isInOut())
+        return false;
+      
+      auto indexTy = index->getInterfaceType()
+                        ->getCanonicalType(sub->getGenericSignatureOfContext());
+      
+      // TODO: Handle reabstraction and tuple explosion in thunk generation.
+      // This wasn't previously a concern because anything that was Hashable
+      // had only one abstraction level and no explosion.
+      
+      if (isa<TupleType>(indexTy))
+        return false;
+      
+      auto indexObjTy = indexTy;
+      if (auto objTy = indexObjTy.getOptionalObjectType())
+        indexObjTy = objTy;
+      
+      if (isa<AnyFunctionType>(indexObjTy)
+          || isa<AnyMetatypeType>(indexObjTy))
+        return false;
+    }
+  }
+
   return true;
 }

lib/SILGen/SILGen.cpp

@@ -1286,20 +1286,17 @@ void SILGenModule::tryEmitPropertyDescriptor(AbstractStorageDecl *decl) {
 
   Type baseTy;
   if (decl->getDeclContext()->isTypeContext()) {
+    // TODO: Static properties should eventually be referenceable as
+    // keypaths from T.Type -> Element, viz `baseTy = MetatypeType::get(baseTy)`
+    assert(!decl->isStatic());
+    
     baseTy = decl->getDeclContext()->getSelfInterfaceType()
                  ->getCanonicalType(decl->getInnermostDeclContext()
                                         ->getGenericSignatureOfContext());
-    if (decl->isStatic()) {
-      // TODO: Static properties should eventually be referenceable as
-      // keypaths from T.Type -> Element
-      //baseTy = MetatypeType::get(baseTy);
-      return;
-    }
   } else {
     // TODO: Global variables should eventually be referenceable as
-    // key paths from ()
-    //baseTy = TupleType::getEmpty(getASTContext());
-    return;
+    // key paths from (), viz. baseTy = TupleType::getEmpty(getASTContext());
+    llvm_unreachable("should not export a property descriptor yet");
   }
 
   auto genericEnv = decl->getInnermostDeclContext()
@@ -1316,33 +1313,6 @@ void SILGenModule::tryEmitPropertyDescriptor(AbstractStorageDecl *decl) {
   if (genericEnv)
     subs = genericEnv->getForwardingSubstitutionMap();
 
-  if (auto sub = dyn_cast<SubscriptDecl>(decl)) {
-    for (auto *index : *sub->getIndices()) {
-      // Keypaths can't capture inout indices.
-      if (index->isInOut())
-        return;
-
-      // TODO: Handle reabstraction and tuple explosion in thunk generation.
-      // This wasn't previously a concern because anything that was Hashable
-      // had only one abstraction level and no explosion.
-      auto indexTy = index->getInterfaceType();
-      
-      if (isa<TupleType>(indexTy->getCanonicalType(sub->getGenericSignature())))
-        return;
-      
-      if (genericEnv)
-        indexTy = genericEnv->mapTypeIntoContext(indexTy);
-      
-      auto indexLoweredTy = Types.getLoweredType(indexTy);
-      auto indexOpaqueLoweredTy =
-        Types.getLoweredType(AbstractionPattern::getOpaque(), indexTy);
-      
-      if (indexOpaqueLoweredTy.getAddressType()
-             != indexLoweredTy.getAddressType())
-        return;
-    }
-  }
-
   auto component = emitKeyPathComponentForDecl(SILLocation(decl),
                                                genericEnv,
                                                baseOperand, needsGenericContext,

lib/SILGen/SILGenExpr.cpp

@@ -3635,7 +3635,13 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
     [&]() -> bool {
       return getASTContext().LangOpts.EnableKeyPathResilience
         && !forPropertyDescriptor
-        && storage->getModuleContext() != SwiftModule;
+        && storage->getModuleContext() != SwiftModule
+        // Protocol requirements don't have nor need property descriptors.
+        && !isa<ProtocolDecl>(storage->getDeclContext())
+        // Properties that only dispatch via ObjC lookup do not have nor need
+        // property descriptors, since the selector identifies the storage.
+        && (!storage->hasAnyAccessors()
+            || !getGetterDeclRef(storage).isForeign);
     };
 
   auto strategy = storage->getAccessStrategy(AccessSemantics::Ordinary,