Implement the final approved syntax for SE-227 identity key paths.

`\.self` is the final chosen syntax. Implement support for this syntax, and remove the stopgap builtin and `WritableKeyPath._identity` property that were in place before.
swiftlang · Sep 19, 2018 · 93b5de6 · 93b5de6
1 parent 86e11c5
commit 93b5de6
Show file tree

Hide file tree

Showing 21 changed files with 115 additions and 110 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,20 @@ CHANGELOG
 Swift 5.0
 ---------
 
+* [SE-0227][]:
+
+  Key paths now support the `\.self` keypath, which is a `WritableKeyPath`
+  that refers to its entire input value:
+
+    ```swift
+    let id = \Int.self
+
+    var x = 2
+    print(x[keyPath: id]) // prints 2
+    x[keyPath: id] = 3
+    print(x[keyPath: id]) // prints 3
+    ```
+
 * [SE-0214][]:
 
   Renamed the `DictionaryLiteral` type to `KeyValuePairs`.

diff --git a/include/swift/AST/Builtins.def b/include/swift/AST/Builtins.def
@@ -373,11 +373,6 @@ BUILTIN_SIL_OPERATION(AllocWithTailElems, "allocWithTailElems", Special)
 /// Projects the first tail-allocated element of type E from a class C.
 BUILTIN_SIL_OPERATION(ProjectTailElems, "projectTailElems", Special)
 
-/// identityKeyPath : <T> () -> WritableKeyPath<T, T>
-///
-/// Creates an identity key path object. (TODO: replace with proper syntax)
-BUILTIN_SIL_OPERATION(IdentityKeyPath, "identityKeyPath", Special)
-
 #undef BUILTIN_SIL_OPERATION
 
 // BUILTIN_RUNTIME_CALL - A call into a runtime function.

diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h
@@ -4773,7 +4773,8 @@ class KeyPathExpr : public Expr {
       Subscript,
       OptionalForce,
       OptionalChain,
-      OptionalWrap
+      OptionalWrap,
+      Identity,
     };
 
   private:
@@ -4787,9 +4788,11 @@ class KeyPathExpr : public Expr {
     } Decl;
 
 
-    llvm::PointerIntPair<Expr *, 3, Kind> SubscriptIndexExprAndKind;
-    ArrayRef<Identifier> SubscriptLabels;
-    ArrayRef<ProtocolConformanceRef> SubscriptHashableConformances;
+    Expr *SubscriptIndexExpr;
+    const Identifier *SubscriptLabelsData;
+    const ProtocolConformanceRef *SubscriptHashableConformancesData;
+    unsigned SubscriptSize;
+    Kind KindValue;
     Type ComponentType;
     SourceLoc Loc;
 
@@ -4914,12 +4917,18 @@ class KeyPathExpr : public Expr {
                        SourceLoc());
     }
 
+    static Component forIdentity(SourceLoc selfLoc) {
+      return Component(nullptr, {}, nullptr, {}, {},
+                       Kind::Identity, Type(),
+                       selfLoc);
+    }
+
     SourceLoc getLoc() const {
       return Loc;
     }
 
     Kind getKind() const {
-      return SubscriptIndexExprAndKind.getInt();
+      return KindValue;
     }
 
     bool isValid() const {
@@ -4936,6 +4945,7 @@ class KeyPathExpr : public Expr {
       case Kind::OptionalWrap:
       case Kind::OptionalForce:
       case Kind::Property:
+      case Kind::Identity:
         return true;
 
       case Kind::UnresolvedSubscript:
@@ -4950,14 +4960,15 @@ class KeyPathExpr : public Expr {
       switch (getKind()) {
       case Kind::Subscript:
       case Kind::UnresolvedSubscript:
-        return SubscriptIndexExprAndKind.getPointer();
+        return SubscriptIndexExpr;
 
       case Kind::Invalid:
       case Kind::OptionalChain:
       case Kind::OptionalWrap:
       case Kind::OptionalForce:
       case Kind::UnresolvedProperty:
       case Kind::Property:
+      case Kind::Identity:
         return nullptr;
       }
       llvm_unreachable("unhandled kind");
@@ -4967,14 +4978,15 @@ class KeyPathExpr : public Expr {
       switch (getKind()) {
       case Kind::Subscript:
       case Kind::UnresolvedSubscript:
-        return SubscriptLabels;
+        return {SubscriptLabelsData, (size_t)SubscriptSize};
 
       case Kind::Invalid:
       case Kind::OptionalChain:
       case Kind::OptionalWrap:
       case Kind::OptionalForce:
       case Kind::UnresolvedProperty:
       case Kind::Property:
+      case Kind::Identity:
         llvm_unreachable("no subscript labels for this kind");
       }
       llvm_unreachable("unhandled kind");
@@ -4984,7 +4996,9 @@ class KeyPathExpr : public Expr {
     getSubscriptIndexHashableConformances() const {
       switch (getKind()) {
       case Kind::Subscript:
-        return SubscriptHashableConformances;
+        if (!SubscriptHashableConformancesData)
+          return {};
+        return {SubscriptHashableConformancesData, (size_t)SubscriptSize};
 
       case Kind::UnresolvedSubscript:
       case Kind::Invalid:
@@ -4993,6 +5007,7 @@ class KeyPathExpr : public Expr {
       case Kind::OptionalForce:
       case Kind::UnresolvedProperty:
       case Kind::Property:
+      case Kind::Identity:
         return {};
       }
       llvm_unreachable("unhandled kind");
@@ -5013,6 +5028,7 @@ class KeyPathExpr : public Expr {
       case Kind::OptionalWrap:
       case Kind::OptionalForce:
       case Kind::Property:
+      case Kind::Identity:
         llvm_unreachable("no unresolved name for this kind");
       }
       llvm_unreachable("unhandled kind");
@@ -5030,6 +5046,7 @@ class KeyPathExpr : public Expr {
       case Kind::OptionalChain:
       case Kind::OptionalWrap:
       case Kind::OptionalForce:
+      case Kind::Identity:
         llvm_unreachable("no decl ref for this kind");
       }
       llvm_unreachable("unhandled kind");

diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
@@ -2597,6 +2597,10 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
         component.getIndexExpr()->print(OS, Indent + 4);
         OS.indent(Indent + 4);
         break;
+      case KeyPathExpr::Component::Kind::Identity:
+        OS << "identity";
+        OS << '\n';
+        break;
       }
       OS << "type=";
       component.getComponentType().print(OS);

diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp
@@ -1034,6 +1034,7 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
       case KeyPathExpr::Component::Kind::Property:
       case KeyPathExpr::Component::Kind::UnresolvedProperty:
       case KeyPathExpr::Component::Kind::Invalid:
+      case KeyPathExpr::Component::Kind::Identity:
         // No subexpr to visit.
         break;
       }

diff --git a/lib/AST/Builtins.cpp b/lib/AST/Builtins.cpp
@@ -991,15 +991,6 @@ static ValueDecl *getTypeJoinMetaOperation(ASTContext &Context, Identifier Id) {
   return builder.build(Id);
 }
 
-static ValueDecl *getIdentityKeyPathOperation(ASTContext &Context,
-                                              Identifier Id) {
-  BuiltinGenericSignatureBuilder builder(Context, 1);
-  auto arg = makeGenericParam();
-  builder.setResult(makeBoundGenericType(Context.getWritableKeyPathDecl(),
-                                         arg, arg));
-  return builder.build(Id);
-}
-
 static ValueDecl *getCanBeObjCClassOperation(ASTContext &Context,
                                              Identifier Id) {
   // <T> T.Type -> Builtin.Int8
@@ -1879,10 +1870,7 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
     return getTypeJoinInoutOperation(Context, Id);
 
   case BuiltinValueKind::TypeJoinMeta:
-    return getTypeJoinMetaOperation(Context, Id);
-
-  case BuiltinValueKind::IdentityKeyPath:
-    return getIdentityKeyPathOperation(Context, Id);
+    return getTypeJoinMetaOperation(Context, Id);      
   }
 
   llvm_unreachable("bad builtin value!");

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
@@ -2135,13 +2135,16 @@ KeyPathExpr::Component::Component(ASTContext *ctxForCopyingLabels,
                      Kind kind,
                      Type type,
                      SourceLoc loc)
-    : Decl(decl), SubscriptIndexExprAndKind(indexExpr, kind),
-      SubscriptLabels(subscriptLabels.empty()
-                       ? subscriptLabels
-                       : ctxForCopyingLabels->AllocateCopy(subscriptLabels)),
-      SubscriptHashableConformances(indexHashables),
+    : Decl(decl), SubscriptIndexExpr(indexExpr), KindValue(kind),
       ComponentType(type), Loc(loc)
-  {}
+{
+  assert(subscriptLabels.size() == indexHashables.size()
+         || indexHashables.empty());
+  SubscriptLabelsData = subscriptLabels.data();
+  SubscriptHashableConformancesData = indexHashables.empty()
+    ? nullptr : indexHashables.data();
+  SubscriptSize = subscriptLabels.size();
+}
 
 KeyPathExpr::Component
 KeyPathExpr::Component::forSubscriptWithPrebuiltIndexExpr(
@@ -2157,8 +2160,10 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(
     ArrayRef<ProtocolConformanceRef> hashables) {
   switch (getKind()) {
   case Kind::Subscript:
-    SubscriptHashableConformances = getComponentType()->getASTContext()
-      .AllocateCopy(hashables);
+    assert(hashables.size() == SubscriptSize);
+    SubscriptHashableConformancesData = getComponentType()->getASTContext()
+      .AllocateCopy(hashables)
+      .data();
     return;
 
   case Kind::UnresolvedSubscript:
@@ -2168,6 +2173,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(
   case Kind::OptionalForce:
   case Kind::UnresolvedProperty:
   case Kind::Property:
+  case Kind::Identity:
     llvm_unreachable("no hashable conformances for this kind");
   }
 }

diff --git a/lib/IDE/SourceEntityWalker.cpp b/lib/IDE/SourceEntityWalker.cpp
@@ -367,6 +367,7 @@ std::pair<bool, Expr *> SemaAnnotator::walkToExprPre(Expr *E) {
       case KeyPathExpr::Component::Kind::OptionalChain:
       case KeyPathExpr::Component::Kind::OptionalWrap:
       case KeyPathExpr::Component::Kind::OptionalForce:
+      case KeyPathExpr::Component::Kind::Identity:
         break;
       }
     }

diff --git a/lib/SILGen/SILGenBuiltin.cpp b/lib/SILGen/SILGenBuiltin.cpp
@@ -970,58 +970,6 @@ static ManagedValue emitBuiltinProjectTailElems(SILGenFunction &SGF,
   return ManagedValue::forUnmanaged(result);
 }
 
-static ManagedValue emitBuiltinIdentityKeyPath(SILGenFunction &SGF,
-                                               SILLocation loc,
-                                               SubstitutionMap subs,
-                                               ArrayRef<ManagedValue> args,
-                                               SGFContext C) {
-  assert(subs.getReplacementTypes().size() == 1 &&
-         "identityKeyPath should have one substitution");
-  assert(args.size() == 0 &&
-         "identityKeyPath should have no args");
-
-  auto identityTy = subs.getReplacementTypes()[0]->getCanonicalType();
-
-  // The `self` key can be used for identity in Cocoa KVC as well.
-  StringRef objcString = SGF.getASTContext().LangOpts.EnableObjCInterop
-    ? "self" : "";
-
-  // The key path pattern has to capture some generic context if the type is
-  // dependent on this generic context. We only need the specific type, though,
-  // not the entire generic environment.
-  bool isDependent = identityTy->hasArchetype();
-  CanType identityPatternTy = identityTy;
-  CanGenericSignature patternSig = nullptr;
-  SubstitutionMap patternSubs;
-  if (isDependent) {
-    auto param = GenericTypeParamType::get(0, 0, SGF.getASTContext());
-    identityPatternTy = param->getCanonicalType();
-    patternSig = GenericSignature::get(param, {})->getCanonicalSignature();
-    patternSubs = SubstitutionMap::get(patternSig,
-                                       llvm::makeArrayRef((Type)identityTy),
-                                       {});
-  }
-
-  auto identityPattern = KeyPathPattern::get(SGF.SGM.M,
-                                             patternSig,
-                                             identityPatternTy,
-                                             identityPatternTy,
-                                             {},
-                                             objcString);
-
-  auto kpTy = BoundGenericType::get(SGF.getASTContext().getWritableKeyPathDecl(),
-                                    Type(),
-                                    {identityTy, identityTy})
-    ->getCanonicalType();
-
-  auto keyPath = SGF.B.createKeyPath(loc, identityPattern,
-                                     patternSubs,
-                                     {},
-                                     SILType::getPrimitiveObjectType(kpTy));
-  return SGF.emitManagedRValueWithCleanup(keyPath);
-}
-
-
 /// Specialized emitter for type traits.
 template<TypeTraitResult (TypeBase::*Trait)(),
          BuiltinValueKind Kind>

diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -3849,6 +3849,9 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
                     KeyPathPatternComponent::forOptional(loweredKind, baseTy));
       break;
     }
+
+    case KeyPathExpr::Component::Kind::Identity:
+      continue;
 
     case KeyPathExpr::Component::Kind::Invalid:
     case KeyPathExpr::Component::Kind::UnresolvedProperty:

diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -276,7 +276,11 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,
     case KeyPathExpr::Component::Kind::OptionalWrap:
       // KVC propagates nulls, so these don't affect the key path string.
       continue;
-
+    case KeyPathExpr::Component::Kind::Identity:
+      // The identity component can be elided from the KVC string (unless it's
+      // the only component, in which case we use @"self").
+      continue;
+
     case KeyPathExpr::Component::Kind::Property: {
       // Property references must be to @objc properties.
       // TODO: If we added special properties matching KVC operators like '@sum',
@@ -307,6 +311,12 @@ static bool buildObjCKeyPathString(KeyPathExpr *E,
     }
   }
 
+  // If there are no non-identity components, this is the "self" key.
+  if (buf.empty()) {
+    auto self = StringRef("self");
+    buf.append(self.begin(), self.end());
+  }
+
   return true;
 }
 
@@ -4514,7 +4524,11 @@ namespace {
           baseTy = component.getComponentType();
           resolvedComponents.push_back(component);
           break;
-
+        case KeyPathExpr::Component::Kind::Identity:
+          component = origComponent;
+          component.setComponentType(baseTy);
+          resolvedComponents.push_back(component);
+          break;
         case KeyPathExpr::Component::Kind::Property:
         case KeyPathExpr::Component::Kind::Subscript:
         case KeyPathExpr::Component::Kind::OptionalWrap:

diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp
@@ -6635,6 +6635,7 @@ static bool diagnoseKeyPathComponents(ConstraintSystem &CS, KeyPathExpr *KPE,
       break;
 
     case KeyPathExpr::Component::Kind::Invalid:
+    case KeyPathExpr::Component::Kind::Identity:
     case KeyPathExpr::Component::Kind::OptionalChain:
     case KeyPathExpr::Component::Kind::OptionalForce:
       // FIXME: Diagnose optional chaining and forcing properly.

diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp
@@ -3067,6 +3067,8 @@ namespace {
           base = OptionalType::get(base);
           break;
         }
+        case KeyPathExpr::Component::Kind::Identity:
+          continue;
         }
       }
 

diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -638,6 +638,7 @@ getCalleeDeclAndArgs(ConstraintSystem &cs,
     case KeyPathExpr::Component::Kind::OptionalForce:
     case KeyPathExpr::Component::Kind::OptionalChain:
     case KeyPathExpr::Component::Kind::OptionalWrap:
+    case KeyPathExpr::Component::Kind::Identity:
       return std::make_tuple(nullptr, 0, argLabels, hasTrailingClosure);
     }
 
@@ -4137,6 +4138,7 @@ ConstraintSystem::simplifyKeyPathConstraint(Type keyPathTy,
 
     switch (component.getKind()) {
     case KeyPathExpr::Component::Kind::Invalid:
+    case KeyPathExpr::Component::Kind::Identity:
       break;
 
     case KeyPathExpr::Component::Kind::Property:
-Original file line number
+Diff line change
@@ Expand Up / @@ -3067,6 +3067,8 @@ namespace { @@
               base = OptionalType::get(base);
               break;
             }
+            case KeyPathExpr::Component::Kind::Identity:
+              continue;
             }
           }
@@ Expand Down @@