SILGen: Lower keypath references to other modules' properties and subscripts as external keypath components.

jckarter · jckarter · commit b00ea61945de · 2018-02-23T12:58:19.000-08:00
This way we'll link against the key path component the other module provides instead of making fragile assumptions about its current implementation. Since external keypath lowering isn't fully implemented elsewhere in the compiler, this is enabled behind a staging flag.

external keypath staging
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -219,6 +219,9 @@ namespace swift {
     /// This is for bootstrapping. It can't be in SILOptions because the
     /// TypeChecker uses it to set resolve the ParameterConvention.
     bool EnableSILOpaqueValues = false;
+    
+    /// Enables key path resilience.
+    bool EnableKeyPathResilience = false;
 
     /// If set to true, the diagnosis engine can assume the emitted diagnostics
     /// will be used in editor. This usually leads to more aggressive fixit.
diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td
@@ -463,4 +463,7 @@ def enable_verify_exclusivity : Flag<["-"], "enable-verify-exclusivity">,
 def disable_verify_exclusivity : Flag<["-"], "disable-verify-exclusivity">,
   HelpText<"Diable verification of access markers used to enforce exclusivity.">;
 
+def enable_key_path_resilience : Flag<["-"], "enable-key-path-resilience">,
+  HelpText<"Enable key path resilience.">;
+
 } // end let Flags = [FrontendOption, NoDriverOption, HelpHidden]
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -326,6 +326,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
                    Target.isOSDarwin());
   Opts.EnableSILOpaqueValues |= Args.hasArg(OPT_enable_sil_opaque_values);
 
+  Opts.EnableKeyPathResilience |= Args.hasArg(OPT_enable_key_path_resilience);
+  
 #if SWIFT_DARWIN_ENABLE_STABLE_ABI_BIT
   Opts.UseDarwinPreStableABIBit = false;
 #else
diff --git a/lib/SIL/SILVerifier.cpp b/lib/SIL/SILVerifier.cpp
@@ -3999,7 +3999,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
           auto substType = component.getExternalDecl()->getStorageInterfaceType()
             .subst(subs);
           require(substType->isEqual(component.getComponentType()),
-                  "component type should make storage type of referenced "
+                  "component type should match storage type of referenced "
                   "declaration");
 
           // Index types should match the lowered index types expected by the
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -3702,6 +3702,96 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
   auto baseTy = rootTy;
   SmallVector<SILValue, 4> operands;
   
+  auto lowerSubscriptIndices =
+    [this, &operands, &needsGenericContext, baseTy, E]
+    (const KeyPathExpr::Component &component) ->
+                                SmallVector<KeyPathPatternComponent::Index, 4> {
+      // Capturing an index value dependent on the generic context means we
+      // need the generic context captured in the key path.
+      needsGenericContext |=
+          component.getIndexExpr()->getType()->hasArchetype()
+        | baseTy->hasTypeParameter();
+      
+      // Evaluate the index arguments.
+      SmallVector<RValue, 2> indexValues;
+      auto indexResult = visit(component.getIndexExpr(), SGFContext());
+      if (isa<TupleType>(indexResult.getType())) {
+        std::move(indexResult).extractElements(indexValues);
+      } else {
+        indexValues.push_back(std::move(indexResult));
+      }
+
+      SmallVector<KeyPathPatternComponent::Index, 4> indexPatterns;
+      for (unsigned i : indices(indexValues)) {
+        auto hashable = component.getSubscriptIndexHashableConformances()[i];
+        assert(hashable.isAbstract() ||
+          hashable.getConcrete()->getType()->isEqual(indexValues[i].getType()));
+        auto &value = indexValues[i];
+        
+        auto indexTy = value.getType()->mapTypeOutOfContext()->getCanonicalType();
+        auto indexLoweredTy = SGF.getLoweredType(value.getType());
+        indexLoweredTy = SILType::getPrimitiveType(
+          indexLoweredTy.getSwiftRValueType()->mapTypeOutOfContext()
+             ->getCanonicalType(),
+          indexLoweredTy.getCategory());
+        indexPatterns.push_back({(unsigned)operands.size(),
+                 indexTy, indexLoweredTy,
+                 hashable});
+        operands.push_back(
+          std::move(indexValues[i]).forwardAsSingleValue(SGF, E));
+      }
+      return indexPatterns;
+    };
+  
+  /// Returns true if a key path component for the given property or
+  /// subscript should be externally referenced.
+  auto shouldUseExternalKeyPathComponent =
+    [&](AbstractStorageDecl *storage) -> bool {
+      return SGF.getASTContext().LangOpts.EnableKeyPathResilience
+        && storage->getModuleContext() != SGF.SGM.SwiftModule;
+    };
+  
+  /// Build an external key path component referencing a property or subscript
+  /// from another module.
+  auto makeExternalKeyPathComponent =
+    [&](const KeyPathExpr::Component &component,
+        CanType ty) -> KeyPathPatternComponent {
+      SmallVector<KeyPathPatternComponent::Index, 4> indices;
+      SubstitutionList subs = component.getDeclRef().getSubstitutions();
+      
+      // Map the substitutions out of context.
+      if (!subs.empty()) {
+        // If any of the substitutions involve local archetypes, then the
+        // key path pattern needs to capture the generic context, and we need
+        // to map the pattern substitutions out of this context.
+        if (std::any_of(subs.begin(), subs.end(),
+                        [](const Substitution &s) -> bool {
+                          return s.getReplacement()->hasArchetype();
+                        })) {
+          needsGenericContext = true;
+          auto sig = component.getDeclRef().getDecl()
+            ->getInnermostDeclContext()
+            ->getGenericSignatureOfContext();
+          auto subMap = sig
+            ->getSubstitutionMap(component.getDeclRef().getSubstitutions());
+          subMap = subMap.mapReplacementTypesOutOfContext();
+          SmallVector<Substitution, 4> subsBuf;
+          
+          sig->getSubstitutions(subMap, subsBuf);
+          
+          subs = SGF.getASTContext().AllocateCopy(subsBuf);
+        }
+      }
+      
+      if (component.getKind() ==  KeyPathExpr::Component::Kind::Subscript)
+        indices = lowerSubscriptIndices(component);
+      return KeyPathPatternComponent::forExternal(
+        cast<AbstractStorageDecl>(component.getDeclRef().getDecl()),
+        subs,
+        SGF.getASTContext().AllocateCopy(indices),
+        ty);
+    };
+  
   for (auto &component : E->getComponents()) {
     switch (auto kind = component.getKind()) {
     case KeyPathExpr::Component::Kind::Property: {
@@ -3711,6 +3801,12 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
         ->getReferenceStorageReferent()
         ->getCanonicalType();
       
+      if (shouldUseExternalKeyPathComponent(decl)) {
+        loweredComponents.push_back(makeExternalKeyPathComponent(component,
+                                                                 baseTy));
+        continue;
+      }
+      
       switch (auto strategy = decl->getAccessStrategy(AccessSemantics::Ordinary,
                                                       AccessKind::ReadWrite)) {
       case AccessStrategy::Storage: {
@@ -3805,44 +3901,16 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
           ->substGenericArgs(component.getDeclRef().getSubstitutions());
       auto baseSubscriptInterfaceTy = cast<AnyFunctionType>(
         baseSubscriptTy->mapTypeOutOfContext()->getCanonicalType());
-      
       baseTy = baseSubscriptInterfaceTy.getResult();
-
-      // Capturing an index value dependent on the generic context means we
-      // need the generic context captured in the key path.
-      needsGenericContext |=
-          component.getIndexExpr()->getType()->hasArchetype()
-        | baseTy->hasTypeParameter();
-      
-      // Evaluate the index arguments.
-      SmallVector<RValue, 2> indexValues;
-      auto indexResult = visit(component.getIndexExpr(), SGFContext());
-      if (isa<TupleType>(indexResult.getType())) {
-        std::move(indexResult).extractElements(indexValues);
-      } else {
-        indexValues.push_back(std::move(indexResult));
+    
+      if (shouldUseExternalKeyPathComponent(decl)) {
+        loweredComponents.push_back(makeExternalKeyPathComponent(component,
+                                                                 baseTy));
+        continue;
       }
-
-      SmallVector<KeyPathPatternComponent::Index, 4> indexPatterns;
+      
+      auto indexPatterns = lowerSubscriptIndices(component);
       SILFunction *indexEquals = nullptr, *indexHash = nullptr;
-      for (unsigned i : indices(indexValues)) {
-        auto hashable = component.getSubscriptIndexHashableConformances()[i];
-        assert(hashable.isAbstract() ||
-          hashable.getConcrete()->getType()->isEqual(indexValues[i].getType()));
-        auto &value = indexValues[i];
-        
-        auto indexTy = value.getType()->mapTypeOutOfContext()->getCanonicalType();
-        auto indexLoweredTy = SGF.getLoweredType(value.getType());
-        indexLoweredTy = SILType::getPrimitiveType(
-          indexLoweredTy.getSwiftRValueType()->mapTypeOutOfContext()
-             ->getCanonicalType(),
-          indexLoweredTy.getCategory());
-        indexPatterns.push_back({(unsigned)operands.size(),
-                 indexTy, indexLoweredTy,
-                 hashable});
-        operands.push_back(
-          std::move(indexValues[i]).forwardAsSingleValue(SGF, E));
-      }
       getOrCreateKeyPathEqualsAndHash(SGF, SILLocation(E),
                needsGenericContext ? SGF.F.getGenericEnvironment() : nullptr,
                indexPatterns,
diff --git a/test/SILGen/Inputs/ExternalKeyPaths.swift b/test/SILGen/Inputs/ExternalKeyPaths.swift
@@ -0,0 +1,5 @@
+public struct External<A> {
+  public var property: A
+  public var intProperty: Int
+  public subscript<B: Hashable>(index: B) -> A { return property }
+}
diff --git a/test/SILGen/external-keypath.swift b/test/SILGen/external-keypath.swift
@@ -0,0 +1,53 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %target-swift-frontend -emit-module -o %t/ExternalKeyPaths.swiftmodule -module-name ExternalKeyPaths %S/Inputs/ExternalKeyPaths.swift
+// RUN: %target-swift-frontend -enable-key-path-resilience -emit-silgen -I %t %s | %FileCheck %s
+
+import ExternalKeyPaths
+
+struct Local {
+  var x: Int
+  var y: String
+}
+
+// CHECK-LABEL: sil hidden @{{.*}}16externalKeyPaths
+func externalKeyPaths<T: Hashable, U>(_ x: T, _ y: U, _ z: Int) {
+  // CHECK: keypath $WritableKeyPath<External<Int>, Int>, (root $External<Int>; external #External.property<Int> : $Int)
+  _ = \External<Int>.property
+
+  // CHECK: keypath $WritableKeyPath<External<Int>, Int>, (root $External<Int>; external #External.intProperty<Int> : $Int)
+  _ = \External<Int>.intProperty
+
+  // CHECK: keypath $WritableKeyPath<External<T>, T>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (root $External<τ_0_0>; external #External.property<T> : $τ_0_0) <T, U> 
+  _ = \External<T>.property
+
+  // CHECK: keypath $WritableKeyPath<External<T>, Int>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (root $External<τ_0_0>; external #External.intProperty<T> : $Int) <T, U>
+  _ = \External<T>.intProperty
+
+  // CHECK: keypath $WritableKeyPath<External<U>, U>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (root $External<τ_0_1>; external #External.property<U> : $τ_0_1) <T, U>
+  _ = \External<U>.property
+
+  // CHECK: keypath $WritableKeyPath<External<U>, Int>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (root $External<τ_0_1>; external #External.intProperty<U> : $Int) <T, U>
+  _ = \External<U>.intProperty
+
+  // CHECK: keypath $KeyPath<External<Int>, Int>, (root $External<Int>; external #External.subscript<Int, Int>[%$0 : $Int : $Int] : $Int) (%2)
+  _ = \External<Int>.[z]
+
+  // CHECK: keypath $KeyPath<External<T>, T>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (root $External<τ_0_0>; external #External.subscript<T, T>[%$0 : $τ_0_0 : $*τ_0_0] : $τ_0_0) <T, U> ({{.*}})
+  _ = \External<T>.[x]
+
+  // CHECK: keypath $KeyPath<External<U>, U>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (root $External<τ_0_1>; external #External.subscript<U, T>[%$0 : $τ_0_0 : $*τ_0_0] : $τ_0_1) <T, U> ({{.*}})
+  _ = \External<U>.[x]
+
+  // CHECK: keypath $KeyPath<External<Local>, Int>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (
+  // CHECK-SAME: root $External<Local>;
+  // CHECK-SAME: external #External.subscript<Local, T>[%$0 : $τ_0_0 : $*τ_0_0] : $Local;
+  // CHECK-SAME: stored_property #Local.x : $Int) <T, U> ({{.*}})
+  _ = \External<Local>.[x].x
+
+  // CHECK: keypath $KeyPath<External<Local>, String>, <τ_0_0, τ_0_1 where τ_0_0 : Hashable> (
+  // CHECK-SAME: root $External<Local>;
+  // CHECK-SAME: external #External.subscript<Local, T>[%$0 : $τ_0_0 : $*τ_0_0] : $Local;
+  // CHECK-SAME: stored_property #Local.y : $String) <T, U> ({{.*}})
+  _ = \External<Local>.[x].y
+}
