Merge pull request swiftlang#27951 from zoecarver/key-path-default-arg

slavapestov · web-flow · commit 7c145fb7be09 · 2019-11-19T00:04:51.000-05:00
Fix KeyPath with default arg
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -6091,6 +6091,39 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e,
   return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL));
 }
 
+SmallVector<ManagedValue, 4> SILGenFunction::emitKeyPathSubscriptOperands(
+    SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr) {
+  Type interfaceType = subscript->getInterfaceType();
+  CanFunctionType substFnType =
+      subs ? cast<FunctionType>(interfaceType->castTo<GenericFunctionType>()
+                                    ->substGenericArgs(subs)
+                                    ->getCanonicalType())
+           : cast<FunctionType>(interfaceType->getCanonicalType());
+  AbstractionPattern origFnType(substFnType);
+  auto fnType =
+      getLoweredType(origFnType, substFnType).castTo<SILFunctionType>();
+
+  SmallVector<ManagedValue, 4> argValues;
+  SmallVector<DelayedArgument, 2> delayedArgs;
+  ArgEmitter emitter(*this, fnType->getRepresentation(),
+                     /*yield*/ false,
+                     /*isForCoroutine*/ false,
+                     ClaimedParamsRef(fnType, fnType->getParameters()),
+                     argValues, delayedArgs,
+                     /*foreign error*/ None, ImportAsMemberStatus());
+
+  auto prepared =
+      prepareSubscriptIndices(subscript, subs,
+                              // Strategy doesn't matter
+                              AccessStrategy::getStorage(), indexExpr);
+  emitter.emitPreparedArgs(std::move(prepared), origFnType);
+
+  if (!delayedArgs.empty())
+    emitDelayedArguments(*this, delayedArgs, argValues);
+
+  return argValues;
+}
+
 ManagedValue ArgumentScope::popPreservingValue(ManagedValue mv) {
   formalEvalScope.pop();
   return normalScope.popPreservingValue(mv);
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -3575,27 +3575,6 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
   
   auto baseTy = rootTy;
   SmallVector<SILValue, 4> operands;
-  
-  auto lowerSubscriptOperands =
-    [this, &operands, E](const KeyPathExpr::Component &component) {
-      if (!component.getIndexExpr())
-        return;
-      
-      // 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));
-      }
-
-      for (auto &rv : indexValues) {
-        operands.push_back(
-          std::move(rv).forwardAsSingleValue(SGF, E));
-      }
-    };
-  
 
   for (auto &component : E->getComponents()) {
     switch (auto kind = component.getKind()) {
@@ -3615,11 +3594,18 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) {
                             component.getSubscriptIndexHashableConformances(),
                             baseTy,
                             /*for descriptor*/ false));
-      lowerSubscriptOperands(component);
-    
-      assert(numOperands == operands.size()
-             && "operand count out of sync");
       baseTy = loweredComponents.back().getComponentType();
+      if (kind == KeyPathExpr::Component::Kind::Property)
+        break;
+
+      auto subscript = cast<SubscriptDecl>(decl);
+      auto loweredArgs = SGF.emitKeyPathSubscriptOperands(
+          subscript, component.getDeclRef().getSubstitutions(),
+          component.getIndexExpr());
+
+      for (auto &arg : loweredArgs) {
+        operands.push_back(arg.forward(SGF));
+      }
 
       break;
     }
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
@@ -688,7 +688,19 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                            SubstitutionMap witnessSubs,
                            IsFreeFunctionWitness_t isFree,
                            bool isSelfConformance);
-  
+
+  /// Generates subscript arguments for keypath. This function handles lowering
+  /// of all index expressions including default arguments.
+  ///
+  /// \returns Lowered index arguments.
+  /// \param subscript - The subscript decl who's arguments are being lowered.
+  /// \param subs - Used to get subscript function type and to substitute generic args.
+  /// \param indexExpr - An expression holding the indices of the
+  /// subscript (either a TupleExpr or a ParenExpr).
+  SmallVector<ManagedValue, 4>
+  emitKeyPathSubscriptOperands(SubscriptDecl *subscript, SubstitutionMap subs,
+                               Expr *indexExpr);
+
   /// Convert a block to a native function with a thunk.
   ManagedValue emitBlockToFunc(SILLocation loc,
                                ManagedValue block,
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -4591,9 +4591,6 @@ namespace {
                               /*applyExpr*/ nullptr, labels,
                               /*hasTrailingClosure*/ false, locator);
 
-      auto component = KeyPathExpr::Component::forSubscriptWithPrebuiltIndexExpr(
-          ref, newIndexExpr, labels, resolvedTy, componentLoc, {});
-
       // We need to be able to hash the captured index values in order for
       // KeyPath itself to be hashable, so check that all of the subscript
       // index components are hashable and collect their conformances here.
@@ -4603,7 +4600,10 @@ namespace {
           cs.getASTContext().getProtocol(KnownProtocolKind::Hashable);
 
       auto fnType = overload.openedType->castTo<FunctionType>();
-      for (const auto &param : fnType->getParams()) {
+      SmallVector<Identifier, 4> newLabels;
+      for (auto &param : fnType->getParams()) {
+        newLabels.push_back(param.getLabel());
+
         auto indexType = simplifyType(param.getPlainType());
         // Index type conformance to Hashable protocol has been
         // verified by the solver, we just need to get it again
@@ -4616,8 +4616,10 @@ namespace {
         conformances.push_back(hashableConformance);
       }
 
-      component.setSubscriptIndexHashableConformances(conformances);
-      return component;
+      return KeyPathExpr::Component::forSubscriptWithPrebuiltIndexExpr(
+          ref, newIndexExpr, cs.getASTContext().AllocateCopy(newLabels),
+          resolvedTy, componentLoc,
+          cs.getASTContext().AllocateCopy(conformances));
     }
 
     Expr *visitKeyPathDotExpr(KeyPathDotExpr *E) {
diff --git a/test/SILGen/Inputs/default_arg_other.swift b/test/SILGen/Inputs/default_arg_other.swift
@@ -0,0 +1,12 @@
+
+public func foo(x: Int = 0) -> Int { x }
+
+public struct Subscript1 {
+  public init() { }
+  public subscript(x: Int = 0) -> Int { x }
+}
+
+public struct Subscript2 {
+  public init() { }
+  public subscript(x: String = #function) -> String { x }
+}
diff --git a/test/SILGen/default_arg_multiple_modules.swift b/test/SILGen/default_arg_multiple_modules.swift
@@ -0,0 +1,53 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -emit-module-path=%t/default_arg_other.swiftmodule -module-name=default_arg_other %S/Inputs/default_arg_other.swift
+// RUN: %target-swift-emit-silgen -module-name default_arg_multiple_modules -I %t %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-runtime
+
+import default_arg_other
+
+// CHECK-LABEL: sil hidden [ossa] @${{.*}}test1{{.*}}
+func test1() -> Int {
+  // CHECK: [[DEF_ARG_FN:%[0-9]+]] = function_ref @$s17default_arg_other3foo1xS2i_tFfA_ : $@convention(thin) () -> Int
+  // CHECK: [[DEF_ARG:%[0-9]+]] = apply [[DEF_ARG_FN]]() {{.*}}
+  // CHECK: [[FN:%[0-9]+]] = function_ref @$s17default_arg_other3foo1xS2i_tF : $@convention(thin) (Int) -> Int
+  // CHECK: [[CALL:%[0-9]+]] = apply [[FN]]([[DEF_ARG]]) {{.*}}
+  // CHECK: return [[CALL]] : $Int
+  return foo()
+}
+
+// CHECK-LABEL: sil hidden [ossa] @${{.*}}test2{{.*}}
+func test2() -> Int {
+  // CHECK: [[DEF_ARG_FN:%[0-9]+]] = function_ref @$s17default_arg_other10Subscript1VyS2icipfA_ : $@convention(thin) () -> Int
+  // CHECK: [[DEF_ARG:%[0-9]+]] = apply [[DEF_ARG_FN]]() {{.*}}
+  // CHECK: [[FN:%[0-9]+]] = function_ref @$s17default_arg_other10Subscript1VyS2icig : $@convention(method) (Int, Subscript1) -> Int
+  // CHECK: [[CALL:%[0-9]+]] = apply [[FN]]([[DEF_ARG]], {{.*}}
+  // CHECK: return [[CALL]] : $Int
+  return Subscript1()[]
+}
+
+// CHECK-LABEL: sil hidden [ossa] @${{.*}}test3{{.*}}
+func test3() -> String {
+  // This should not call default arg constructor
+  // CHECK: [[STR_LIT:%[0-9]+]] = string_literal utf8 "test3()"
+  // CHECK: [[DEF_ARG:%[0-9]+]] = apply %{{[0-9]+}}([[STR_LIT]], {{.*}}
+  // CHECK: [[FN:%[0-9]+]] = function_ref @$s17default_arg_other10Subscript2VyS2Scig : $@convention(method) (@guaranteed String, Subscript2) -> @owned String
+  // CHECK: [[CALL:%[0-9]+]] = apply [[FN]]([[DEF_ARG]], {{.*}}
+  // CHECK: return [[CALL]] : $String
+  return Subscript2()[]
+}
+
+// CHECK-LABEL: sil hidden [ossa] @${{.*}}test4{{.*}}
+func test4() {
+  // CHECK: [[DEF_ARG_FN:%[0-9]+]] = function_ref @$s17default_arg_other10Subscript1VyS2icipfA_ : $@convention(thin) () -> Int
+  // CHECK: [[DEF_ARG:%[0-9]+]] = apply [[DEF_ARG_FN]]() {{.*}}
+  // CHECK: keypath $KeyPath<Subscript1, Int>, (root $Subscript1; gettable_property $Int, id @$s17default_arg_other10Subscript1VyS2icig : $@convention(method) (Int, Subscript1) -> Int, getter @$s17default_arg_other10Subscript1VyS2icipACTK : $@convention(thin) (@in_guaranteed Subscript1, UnsafeRawPointer) -> @out Int, indices [%$0 : $Int : $Int], indices_equals @$sSiTH : $@convention(thin) (UnsafeRawPointer, UnsafeRawPointer) -> Bool, indices_hash @$sSiTh : $@convention(thin) (UnsafeRawPointer) -> Int, external #Subscript1.subscript) ([[DEF_ARG]])
+  _ = \Subscript1.[]
+}
+
+// CHECK-LABEL: sil hidden [ossa] @${{.*}}test5{{.*}}
+func test5() {
+  // This should not call default arg constructor
+  // CHECK: [[STR_LIT:%[0-9]+]] = string_literal utf8 "test5()"
+  // CHECK: [[DEF_ARG:%[0-9]+]] = apply %{{[0-9]+}}([[STR_LIT]], {{.*}}
+  // CHECK: keypath $KeyPath<Subscript2, String>, (root $Subscript2; gettable_property $String, id @$s17default_arg_other10Subscript2VyS2Scig : $@convention(method) (@guaranteed String, Subscript2) -> @owned String, getter @$s17default_arg_other10Subscript2VyS2ScipACTK : $@convention(thin) (@in_guaranteed Subscript2, UnsafeRawPointer) -> @out String, indices [%$0 : $String : $String], indices_equals @$sSSTH : $@convention(thin) (UnsafeRawPointer, UnsafeRawPointer) -> Bool, indices_hash @$sSSTh : $@convention(thin) (UnsafeRawPointer) -> Int, external #Subscript2.subscript) ([[DEF_ARG]])
+  _ = \Subscript2.[]
+}
diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift
@@ -322,7 +322,54 @@ struct Subscripts<T> {
   }
 }
 
-// CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts
+struct SubscriptDefaults1 {
+  subscript(x: Int = 0) -> Int {
+    get { fatalError() }
+    set { fatalError() }
+  }
+  subscript(x: Int, y: Int, z: Int = 0) -> Int {
+    get { fatalError() }
+    set { fatalError() }
+  }
+  subscript(x: Bool, bool y: Bool = false) -> Bool {
+    get { fatalError() }
+    set { fatalError() }
+  }
+  subscript(bool x: Bool, y: Int, z: Int = 0) -> Int {
+    get { fatalError() }
+    set { fatalError() }
+  }
+}
+
+struct SubscriptDefaults2 {
+  subscript(x: Int? = nil) -> Int {
+    get { fatalError() }
+    set { fatalError() }
+  }
+}
+
+struct SubscriptDefaults3 {
+  subscript(x: Int = #line) -> Int {
+    get { fatalError() }
+    set { fatalError() }
+  }
+}
+
+struct SubscriptDefaults4 {
+  subscript<T : Numeric>(x x: T, y y: T = 0) -> T {
+    get { fatalError() }
+    set { fatalError() }
+  }
+}
+
+struct SubscriptDefaults5 {
+  subscript<T : ExpressibleByStringLiteral>(x x: T, y y: T =  #function) -> T {
+    get { fatalError() }
+    set { fatalError() }
+  }
+}
+
+// CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts1x1y1syx_q_SStSHRzSHR_r0_lF
 func subscripts<T: Hashable, U: Hashable>(x: T, y: U, s: String) {
   _ = \Subscripts<T>.[]
   _ = \Subscripts<T>.[generic: x]
@@ -352,6 +399,57 @@ func subscripts<T: Hashable, U: Hashable>(x: T, y: U, s: String) {
 
   _ = \Subscripts<T>.[Bass()]
   _ = \Subscripts<T>.[Treble()]
+
+  _ = \SubscriptDefaults1.[]
+  _ = \SubscriptDefaults1.[0]
+  _ = \SubscriptDefaults1.[0, 0]
+  _ = \SubscriptDefaults1.[0, 0, 0]
+  _ = \SubscriptDefaults1.[false]
+  _ = \SubscriptDefaults1.[false, bool: false]
+  _ = \SubscriptDefaults1.[bool: false, 0]
+  _ = \SubscriptDefaults1.[bool: false, 0, 0]
+  
+  _ = \SubscriptDefaults2.[]
+  _ = \SubscriptDefaults2.[0]
+  _ = \SubscriptDefaults3.[]
+  _ = \SubscriptDefaults3.[0]
+  _ = \SubscriptDefaults4.[x: 0]
+  _ = \SubscriptDefaults4.[x: 0, y: 0]
+  _ = \SubscriptDefaults5.[x: ""]
+  _ = \SubscriptDefaults5.[x: "", y: ""]
+}
+
+// CHECK-LABEL: sil hidden [ossa] @{{.*}}check_default_subscripts
+func check_default_subscripts() {
+  // CHECK: [[INTX:%[0-9]+]] = integer_literal $Builtin.IntLiteral, 0
+  // CHECK: [[IX:%[0-9]+]] = apply %{{[0-9]+}}([[INTX]], {{.*}}
+  // CHECK: [[INTY:%[0-9]+]] = integer_literal $Builtin.IntLiteral, 0
+  // CHECK: [[IY:%[0-9]+]] = apply %{{[0-9]+}}([[INTY]], {{.*}}
+  // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath<SubscriptDefaults4, Int>, (root $SubscriptDefaults4; settable_property $Int, id @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluig : $@convention(method) <τ_0_0 where τ_0_0 : Numeric> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults4) -> @out τ_0_0, getter @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTK : $@convention(thin) (@in_guaranteed SubscriptDefaults4, UnsafeRawPointer) -> @out Int, setter @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTk : $@convention(thin) (@in_guaranteed Int, @inout SubscriptDefaults4, UnsafeRawPointer) -> (), indices [%$0 : $Int : $Int, %$1 : $Int : $Int], indices_equals @$sS2iTH : $@convention(thin) (UnsafeRawPointer, UnsafeRawPointer) -> Bool, indices_hash @$sS2iTh : $@convention(thin) (UnsafeRawPointer) -> Int) ([[IX]], [[IY]])
+  _ = \SubscriptDefaults4.[x: 0, y: 0]
+
+  // CHECK: [[INTINIT:%[0-9]+]] = integer_literal $Builtin.IntLiteral, 0
+  // CHECK: [[I:%[0-9]+]] = apply %{{[0-9]+}}([[INTINIT]], {{.*}}
+  // CHECK: [[DFN:%[0-9]+]] = function_ref @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluipfA0_ : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric> () -> @out τ_0_0
+  // CHECK: [[ALLOC:%[0-9]+]] = alloc_stack $Int
+  // CHECK: apply [[DFN]]<Int>([[ALLOC]]) : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric> () -> @out τ_0_0
+  // CHECK: [[LOAD:%[0-9]+]] = load [trivial] [[ALLOC]] : $*Int
+  // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath<SubscriptDefaults4, Int>, (root $SubscriptDefaults4; settable_property $Int, id @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluig : $@convention(method) <τ_0_0 where τ_0_0 : Numeric> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults4) -> @out τ_0_0, getter @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTK : $@convention(thin) (@in_guaranteed SubscriptDefaults4, UnsafeRawPointer) -> @out Int, setter @$s8keypaths18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTk : $@convention(thin) (@in_guaranteed Int, @inout SubscriptDefaults4, UnsafeRawPointer) -> (), indices [%$0 : $Int : $Int, %$1 : $Int : $Int], indices_equals @$sS2iTH : $@convention(thin) (UnsafeRawPointer, UnsafeRawPointer) -> Bool, indices_hash @$sS2iTh : $@convention(thin) (UnsafeRawPointer) -> Int) ([[I]], [[LOAD]])
+  _ = \SubscriptDefaults4.[x: 0]
+  
+  // CHECK: [[STRX_LIT:%[0-9]+]] = string_literal utf8 ""
+  // CHECK: [[STRX:%[0-9]+]] = apply %{{[0-9]+}}([[STRX_LIT]], {{.*}}
+  // CHECK: [[STRY_LIT:%[0-9]+]] = string_literal utf8 "check_default_subscripts()"
+  // CHECK: [[DEF_ARG:%[0-9]+]] = apply %{{[0-9]+}}([[STRY_LIT]], {{.*}}
+  // CHECK: keypath $WritableKeyPath<SubscriptDefaults5, String>, (root $SubscriptDefaults5; settable_property $String, id @$s8keypaths18SubscriptDefaults5V1x1yxx_xtcs26ExpressibleByStringLiteralRzluig : $@convention(method) <τ_0_0 where τ_0_0 : ExpressibleByStringLiteral> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults5) -> @out τ_0_0, getter @$s8keypaths18SubscriptDefaults5V1x1yxx_xtcs26ExpressibleByStringLiteralRzluipACSSTK : $@convention(thin) (@in_guaranteed SubscriptDefaults5, UnsafeRawPointer) -> @out String, setter @$s8keypaths18SubscriptDefaults5V1x1yxx_xtcs26ExpressibleByStringLiteralRzluipACSSTk : $@convention(thin) (@in_guaranteed String, @inout SubscriptDefaults5, UnsafeRawPointer) -> (), indices [%$0 : $String : $String, %$1 : $String : $String], indices_equals @$sS2STH : $@convention(thin) (UnsafeRawPointer, UnsafeRawPointer) -> Bool, indices_hash @$sS2STh : $@convention(thin) (UnsafeRawPointer) -> Int) ([[STRX]], [[DEF_ARG]])
+  _ = \SubscriptDefaults5.[x: ""]
+  
+  // CHECK: [[STRX_LIT:%[0-9]+]] = string_literal utf8 ""
+  // CHECK: [[STRX:%[0-9]+]] = apply %{{[0-9]+}}([[STRX_LIT]], {{.*}}
+  // CHECK: [[STRY_LIT:%[0-9]+]] = string_literal utf8 ""
+  // CHECK: [[STRY:%[0-9]+]] = apply %{{[0-9]+}}([[STRY_LIT]], {{.*}}
+  // CHECK: keypath $WritableKeyPath<SubscriptDefaults5, String>, (root $SubscriptDefaults5; settable_property $String, id @$s8keypaths18SubscriptDefaults5V1x1yxx_xtcs26ExpressibleByStringLiteralRzluig : $@convention(method) <τ_0_0 where τ_0_0 : ExpressibleByStringLiteral> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults5) -> @out τ_0_0, getter @$s8keypaths18SubscriptDefaults5V1x1yxx_xtcs26ExpressibleByStringLiteralRzluipACSSTK : $@convention(thin) (@in_guaranteed SubscriptDefaults5, UnsafeRawPointer) -> @out String, setter @$s8keypaths18SubscriptDefaults5V1x1yxx_xtcs26ExpressibleByStringLiteralRzluipACSSTk : $@convention(thin) (@in_guaranteed String, @inout SubscriptDefaults5, UnsafeRawPointer) -> (), indices [%$0 : $String : $String, %$1 : $String : $String], indices_equals @$sS2STH : $@convention(thin) (UnsafeRawPointer, UnsafeRawPointer) -> Bool, indices_hash @$sS2STh : $@convention(thin) (UnsafeRawPointer) -> Int) ([[STRX]], [[STRY]])
+  _ = \SubscriptDefaults5.[x: "", y: ""]
 }
 
 // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics
