SILGen: Emit property descriptors for (some) decls that need them.

If a property or subscript is referenceable from other modules, we need to give it a descriptor so that we can reliably build an equivalent key path in or out of that module.

There are some cases that we should handle but don't yet:

- Global and static properties ought to be key-path-able someday, so we should make descriptors for them, but this might need a new key path component kind.
- Subscripts with indexes that aren't Hashable in the current module ought to get descriptors too, in case we ever support non-hashable key path components, and also because a generic subscript might be substituted with Hashable types by an external user, or an external module might post-hoc extend a type to be Hashable, so we really need to change things so that the client supplies the hashing and equality implementations for the indexes instead of the descriptor.

Author: jckarter

include/swift/SIL/SILInstruction.h

@@ -2353,9 +2353,6 @@ class KeyPathPatternComponent {
       Indices(indices),
       IndexEquality{indicesEqual, indicesHash},
       ComponentType(ComponentType) {
-    assert(indices.empty() == !indicesEqual
-           && indices.empty() == !indicesHash
-           && "must have equals/hash functions iff there are indices");
   }
 
   KeyPathPatternComponent(AbstractStorageDecl *externalStorage,

lib/SILGen/SILGen.cpp

@@ -15,6 +15,7 @@
 #include "Scope.h"
 #include "swift/Strings.h"
 #include "swift/AST/DiagnosticsSIL.h"
+#include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/PrettyStackTrace.h"
@@ -1204,7 +1205,69 @@ static bool doesPropertyNeedDescriptor(AbstractStorageDecl *decl) {
 }
 
 void SILGenModule::tryEmitPropertyDescriptor(AbstractStorageDecl *decl) {
-  // TODO
+  // TODO: Key path code emission doesn't handle opaque values properly yet.
+  if (!SILModuleConventions(M).useLoweredAddresses())
+    return;
+  
+  if (!doesPropertyNeedDescriptor(decl))
+    return;
+  
+  auto genericEnv = decl->getInnermostDeclContext()
+                        ->getGenericEnvironmentOfContext();
+  unsigned baseOperand = 0;
+  bool needsGenericContext = true;
+  
+  Type baseTy;
+  if (decl->getDeclContext()->isTypeContext()) {
+    baseTy = decl->getDeclContext()->getSelfInterfaceType();
+    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;
+  }
+  
+  SubstitutionList subs = {};
+  if (genericEnv)
+    subs = genericEnv->getForwardingSubstitutions();
+  
+  // TODO: The hashable conformances for the indices need to be provided by the
+  // client, since they may be post-hoc conformances, or a generic subscript
+  // may be invoked with hashable substitutions. We may eventually allow for
+  // non-hashable keypaths as well.
+  SmallVector<ProtocolConformanceRef, 4> indexHashables;
+  if (auto sub = dyn_cast<SubscriptDecl>(decl)) {
+    auto hashable = getASTContext().getProtocol(KnownProtocolKind::Hashable);
+    for (auto *index : *sub->getIndices()) {
+      if (index->isInOut())
+        return;
+      auto indexTy = index->getInterfaceType();
+      if (genericEnv)
+        indexTy = genericEnv->mapTypeIntoContext(indexTy);
+      
+      auto conformance = sub->getModuleContext()
+                            ->lookupConformance(indexTy, hashable);
+      if (!conformance)
+        return;
+      if (!conformance->getConditionalRequirements().empty())
+        return;
+      indexHashables.push_back(*conformance);
+    }
+  }
+  
+  auto component = emitKeyPathComponentForDecl(SILLocation(decl),
+                                               genericEnv,
+                                               baseOperand, needsGenericContext,
+                                               subs, decl, indexHashables,
+                                               baseTy->getCanonicalType());
+  
+  (void)SILProperty::create(M, /*serialized*/ false, decl, component);
 }
 
 void SILGenModule::emitPropertyBehavior(VarDecl *vd) {

lib/SILGen/SILGen.h

@@ -339,6 +339,16 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
                                  AccessKind accessKind);
   SILDeclRef getMaterializeForSetDeclRef(AbstractStorageDecl *decl);
 
+  KeyPathPatternComponent
+  emitKeyPathComponentForDecl(SILLocation loc,
+                              GenericEnvironment *genericEnv,
+                              unsigned &baseOperand,
+                              bool &needsGenericContext,
+                              SubstitutionList subs,
+                              AbstractStorageDecl *storage,
+                              ArrayRef<ProtocolConformanceRef> indexHashables,
+                              CanType baseTy);
+
   /// Known functions for bridging.
   SILDeclRef getStringToNSStringFn();
   SILDeclRef getNSStringToStringFn();

lib/SILGen/SILGenApply.cpp

@@ -5011,6 +5011,9 @@ ArgumentSource SILGenFunction::prepareAccessorBaseArg(SILLocation loc,
                                                       ManagedValue base,
                                                       CanType baseFormalType,
                                                       SILDeclRef accessor) {
+  if (!base)
+    return ArgumentSource();
+
   AccessorBaseArgPreparer Preparer(*this, loc, base, baseFormalType, accessor);
   return Preparer.prepare();
 }

lib/SILGen/SILGenExpr.cpp

@@ -2983,6 +2983,11 @@ emitKeyPathRValueBase(SILGenFunction &subSGF,
                       CanType &baseType,
                       SubstitutionList &subs,
                       SmallVectorImpl<Substitution> &subsBuf) {
+  // If the storage is at global scope, then the base value () is a formality.
+  // There no real argument to pass to the underlying accessors.
+  if (!storage->getDeclContext()->isTypeContext())
+    return ManagedValue();
+  
   auto paramOrigValue = subSGF.emitManagedRValueWithCleanup(paramArg);
   auto paramSubstValue = subSGF.emitOrigToSubstValue(loc, paramOrigValue,
                                              AbstractionPattern::getOpaque(),
@@ -3147,15 +3152,15 @@ static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM,
 
   SmallVector<Substitution, 2> subsBuf;
 
-  auto paramSubstValue = emitKeyPathRValueBase(subSGF, property,
+  auto baseSubstValue = emitKeyPathRValueBase(subSGF, property,
                                                loc, baseArg,
                                                baseType, subs, subsBuf);
 
   RValue indexValue = loadIndexValuesForKeyPathComponent(subSGF, loc,
                                                          indexes,
                                                          indexPtrArg);
 
-  auto resultSubst = subSGF.emitRValueForStorageLoad(loc, paramSubstValue,
+  auto resultSubst = subSGF.emitRValueForStorageLoad(loc, baseSubstValue,
                                    baseType, /*super*/false,
                                    property, std::move(indexValue),
                                    subs, AccessSemantics::Ordinary,
@@ -3448,6 +3453,11 @@ getOrCreateKeyPathEqualsAndHash(SILGenModule &SGM,
       auto equatable = *subMap
         .lookupConformance(CanType(hashableSig->getGenericParams()[0]),
                            equatableProtocol);
+      
+      assert(equatable.isAbstract() == hashable.isAbstract());
+      if (equatable.isConcrete())
+        assert(equatable.getConcrete()->getType()->isEqual(
+                  hashable.getConcrete()->getType()));
       auto equatableSub = Substitution(formalTy,
                    C.AllocateCopy(ArrayRef<ProtocolConformanceRef>(equatable)));
 
@@ -3690,15 +3700,21 @@ lowerKeyPathSubscriptIndexPatterns(
                  bool &needsGenericContext) {
   // Capturing an index value dependent on the generic context means we
   // need the generic context captured in the key path.
-  auto subMap =
-            subscript->getGenericSignature()->getSubstitutionMap(subscriptSubs);
-  auto subscriptSubstTy = subscript->getInterfaceType().subst(subMap);
+  auto subscriptSubstTy = subscript->getInterfaceType();
+  SubstitutionMap subMap;
+  auto sig = subscript->getGenericSignature();
+  if (sig) {
+    subMap = sig->getSubstitutionMap(subscriptSubs);
+    subscriptSubstTy = subscriptSubstTy.subst(subMap);
+  }
   needsGenericContext |= subscriptSubstTy->hasArchetype();
 
   unsigned i = 0;
   for (auto *index : *subscript->getIndices()) {
-    auto indexTy = index->getInterfaceType().subst(subMap)
-                        ->getCanonicalType();
+    auto indexTy = index->getInterfaceType();
+    if (sig) {
+      indexTy = indexTy.subst(subMap);
+    }
     auto hashable = indexHashables[i++];
     assert(hashable.isAbstract() ||
            hashable.getConcrete()->getType()->isEqual(indexTy));
@@ -3718,20 +3734,24 @@ lowerKeyPathSubscriptIndexPatterns(
   }
 };
 
-static KeyPathPatternComponent
-emitKeyPathComponentForDecl(SILGenModule &SGM,
-                            SILLocation loc,
-                            GenericEnvironment *genericEnv,
-                            unsigned &baseOperand,
-                            bool &needsGenericContext,
-                            SubstitutionList subs,
-                            AbstractStorageDecl *storage,
-                            ArrayRef<ProtocolConformanceRef> indexHashables,
-                            CanType baseTy) {
+KeyPathPatternComponent
+SILGenModule::emitKeyPathComponentForDecl(SILLocation loc,
+                                GenericEnvironment *genericEnv,
+                                unsigned &baseOperand,
+                                bool &needsGenericContext,
+                                SubstitutionList subs,
+                                AbstractStorageDecl *storage,
+                                ArrayRef<ProtocolConformanceRef> indexHashables,
+                                CanType baseTy) {
   if (auto var = dyn_cast<VarDecl>(storage)) {
-    auto componentTy = baseTy->getTypeOfMember(SGM.SwiftModule, var)
-      ->getReferenceStorageReferent()
-      ->getCanonicalType();
+    CanType componentTy;
+    if (!var->getDeclContext()->isTypeContext()) {
+      componentTy = storage->getStorageInterfaceType()->getCanonicalType();
+    } else {
+      componentTy = baseTy->getTypeOfMember(SwiftModule, var)
+        ->getReferenceStorageReferent()
+        ->getCanonicalType();
+    }
 
     switch (auto strategy = var->getAccessStrategy(AccessSemantics::Ordinary,
                                                     AccessKind::ReadWrite)) {
@@ -3741,9 +3761,9 @@ emitKeyPathComponentForDecl(SILGenModule &SGM,
       auto componentObjTy = componentTy->getWithoutSpecifierType();
       if (genericEnv)
         componentObjTy = genericEnv->mapTypeIntoContext(componentObjTy);
-      auto storageTy = SGM.Types.getSubstitutedStorageType(var,
+      auto storageTy = Types.getSubstitutedStorageType(var,
                                                            componentObjTy);
-      auto opaqueTy = SGM.Types
+      auto opaqueTy = Types
         .getLoweredType(AbstractionPattern::getOpaque(), componentObjTy);
 
       if (storageTy.getAddressType() == opaqueTy.getAddressType()) {
@@ -3756,17 +3776,17 @@ emitKeyPathComponentForDecl(SILGenModule &SGM,
     case AccessStrategy::DispatchToAccessor: {
       // We need thunks to bring the getter and setter to the right signature
       // expected by the key path runtime.
-      auto id = getIdForKeyPathComponentComputedProperty(SGM, var,
+      auto id = getIdForKeyPathComponentComputedProperty(*this, var,
                                                          strategy);
-      auto getter = getOrCreateKeyPathGetter(SGM, loc,
+      auto getter = getOrCreateKeyPathGetter(*this, loc,
                var, subs,
                strategy,
                needsGenericContext ? genericEnv : nullptr,
                {},
                baseTy, componentTy);
 
       if (var->isSettable(var->getDeclContext())) {
-        auto setter = getOrCreateKeyPathSetter(SGM, loc,
+        auto setter = getOrCreateKeyPathSetter(*this, loc,
                var, subs,
                strategy,
                needsGenericContext ? genericEnv : nullptr,
@@ -3796,28 +3816,30 @@ emitKeyPathComponentForDecl(SILGenModule &SGM,
     auto componentTy = baseSubscriptInterfaceTy.getResult();
 
     SmallVector<KeyPathPatternComponent::Index, 4> indexPatterns;
-    lowerKeyPathSubscriptIndexPatterns(SGM, indexPatterns,
+    lowerKeyPathSubscriptIndexPatterns(*this, indexPatterns,
                                        decl, subs, indexHashables,
                                        baseOperand,
                                        needsGenericContext);
 
     SILFunction *indexEquals = nullptr, *indexHash = nullptr;
-    getOrCreateKeyPathEqualsAndHash(SGM, loc,
+    // TODO: Property descriptors for external key paths should get their
+    // equality and hashing from the client.
+    getOrCreateKeyPathEqualsAndHash(*this, loc,
              needsGenericContext ? genericEnv : nullptr,
              indexPatterns,
              indexEquals, indexHash);
 
-    auto id = getIdForKeyPathComponentComputedProperty(SGM, decl, strategy);
-    auto getter = getOrCreateKeyPathGetter(SGM, loc,
+    auto id = getIdForKeyPathComponentComputedProperty(*this, decl, strategy);
+    auto getter = getOrCreateKeyPathGetter(*this, loc,
              decl, subs,
              strategy,
              needsGenericContext ? genericEnv : nullptr,
              indexPatterns,
              baseTy, componentTy);
 
-    auto indexPatternsCopy = SGM.getASTContext().AllocateCopy(indexPatterns);
+    auto indexPatternsCopy = getASTContext().AllocateCopy(indexPatterns);
     if (decl->isSettable()) {
-      auto setter = getOrCreateKeyPathSetter(SGM, loc,
+      auto setter = getOrCreateKeyPathSetter(*this, loc,
              decl, subs,
              strategy,
              needsGenericContext ? genericEnv : nullptr,
@@ -3952,7 +3974,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
       } else {
         unsigned numOperands = operands.size();
         loweredComponents.push_back(
-          emitKeyPathComponentForDecl(SGF.SGM, SILLocation(E),
+          SGF.SGM.emitKeyPathComponentForDecl(SILLocation(E),
                               SGF.F.getGenericEnvironment(),
                               numOperands,
                               needsGenericContext,

lib/Serialization/DeserializeSIL.cpp

@@ -2607,7 +2607,8 @@ SILVTable *SILDeserializer::readVTable(DeclID VId) {
   // Another SIL_VTABLE record means the end of this VTable.
   while (kind != SIL_VTABLE && kind != SIL_WITNESS_TABLE &&
          kind != SIL_DEFAULT_WITNESS_TABLE &&
-         kind != SIL_FUNCTION) {
+         kind != SIL_FUNCTION &&
+         kind != SIL_PROPERTY) {
     assert(kind == SIL_VTABLE_ENTRY &&
            "Content of Vtable should be in SIL_VTABLE_ENTRY.");
     ArrayRef<uint64_t> ListOfValues;

test/SILGen/keypath_property_descriptors.swift

@@ -0,0 +1,80 @@
+// RUN: %target-swift-frontend -emit-silgen %s | %FileCheck %s
+
+// TODO: globals should get descriptors
+public var a: Int = 0
+
+@_inlineable
+public var b: Int { return 0 }
+
+@_versioned
+internal var c: Int = 0
+
+// no descriptor
+// CHECK-NOT: sil_property #d
+internal var d: Int = 0
+// CHECK-NOT: sil_property #e
+private var e: Int = 0
+
+public struct A {
+  // CHECK-LABEL: sil_property #A.a
+  public var a: Int = 0
+
+  // CHECK-LABEL: sil_property #A.b
+  @_inlineable
+  public var b: Int { return 0 }
+
+  // CHECK-LABEL: sil_property #A.c
+  @_versioned
+  internal var c: Int = 0
+
+  // no descriptor
+  // CHECK-NOT: sil_property #A.d
+  internal var d: Int = 0
+  // CHECK-NOT: sil_property #A.e
+  fileprivate var e: Int = 0
+  // CHECK-NOT: sil_property #A.f
+  private var f: Int = 0
+
+  // TODO: static vars should get descriptors
+  public static var a: Int = 0
+  @_inlineable
+  public static var b: Int { return 0 }
+  @_versioned
+  internal static var c: Int = 0
+
+  // no descriptor
+  // CHECK-NOT: sil_property #A.d
+  internal static var d: Int = 0
+  // CHECK-NOT: sil_property #A.e
+  fileprivate static var e: Int = 0
+  // CHECK-NOT: sil_property #A.f
+  private static var f: Int = 0
+
+  // CHECK-LABEL: sil_property #A.subscript
+  public subscript(a x: Int) -> Int { return x }
+  // CHECK-LABEL: sil_property #A.subscript
+  @_inlineable
+  public subscript(b x: Int) -> Int { return x }
+  // CHECK-LABEL: sil_property #A.subscript
+  @_versioned
+  internal subscript(c x: Int) -> Int { return x }
+  
+  // no descriptor
+  // CHECK-NOT: sil_property #A.subscript
+  internal subscript(d x: Int) -> Int { return x }
+  fileprivate subscript(e x: Int) -> Int { return x }
+  private subscript(f x: Int) -> Int { return x }
+
+  // TODO: Subscripts with non-hashable subscripts should get descriptors
+  public subscript<T>(a x: T) -> T { return x }
+  @_inlineable
+  public subscript<T>(b x: T) -> T { return x }
+  @_versioned
+  internal subscript<T>(c x: T) -> T { return x }
+  
+  // no descriptor
+  internal subscript<T>(d x: T) -> T { return x }
+  fileprivate subscript<T>(e x: T) -> T { return x }
+  private subscript<T>(f x: T) -> T { return x }
+}
+