From e87d243d701752fff80067a70be58e48bdc72299 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 30 Oct 2019 17:31:00 -0700 Subject: [PATCH 01/29] Fix support for subscript default arguments in KeyPath --- include/swift/AST/Expr.h | 10 +++++++--- lib/AST/Expr.cpp | 4 ++-- lib/SILGen/SILGenExpr.cpp | 25 ++++++++++++++++++++++++- test/SILGen/keypaths.swift | 29 +++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 59500a36d3844..76bd95666d0bd 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -4916,7 +4916,10 @@ class KeyPathExpr : public Expr { const ProtocolConformanceRef *SubscriptHashableConformancesData; union { - unsigned SubscriptSize; + struct { + unsigned numConformances; + unsigned numLabels; + } SubscriptSize; unsigned TupleIndex; }; @@ -5122,7 +5125,7 @@ class KeyPathExpr : public Expr { switch (getKind()) { case Kind::Subscript: case Kind::UnresolvedSubscript: - return {SubscriptLabelsData, (size_t)SubscriptSize}; + return {SubscriptLabelsData, (size_t)SubscriptSize.numLabels}; case Kind::Invalid: case Kind::OptionalChain: @@ -5143,7 +5146,8 @@ class KeyPathExpr : public Expr { case Kind::Subscript: if (!SubscriptHashableConformancesData) return {}; - return {SubscriptHashableConformancesData, (size_t)SubscriptSize}; + return {SubscriptHashableConformancesData, + (size_t)SubscriptSize.numConformances}; case Kind::UnresolvedSubscript: case Kind::Invalid: diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 584d1c54a8eb8..724dabf969477 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2159,7 +2159,7 @@ KeyPathExpr::Component::Component(ASTContext *ctxForCopyingLabels, SubscriptLabelsData = subscriptLabels.data(); SubscriptHashableConformancesData = indexHashables.empty() ? nullptr : indexHashables.data(); - SubscriptSize = subscriptLabels.size(); + SubscriptSize.numLabels = subscriptLabels.size(); } KeyPathExpr::Component @@ -2176,7 +2176,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances( ArrayRef hashables) { switch (getKind()) { case Kind::Subscript: - assert(hashables.size() == SubscriptSize); + SubscriptSize.numConformances = hashables.size(); SubscriptHashableConformancesData = getComponentType()->getASTContext() .AllocateCopy(hashables) .data(); diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 30ccec9ef7cf4..3bb067e40aeef 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3549,7 +3549,30 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { [this, &operands, E](const KeyPathExpr::Component &component) { if (!component.getIndexExpr()) return; - + + if (auto *parenExpr = dyn_cast(component.getIndexExpr())) { + if (auto *defaultArg = + dyn_cast(parenExpr->getSubExpr())) { + const ParamDecl *defaultParam = getParameterAt( + cast(defaultArg->getDefaultArgsOwner().getDecl()), 0); + parenExpr->setSubExpr(defaultParam->getDefaultValue()); + } + } + + if (auto *tupleExpr = dyn_cast(component.getIndexExpr())) { + size_t count = 0; + for (auto *element : tupleExpr->getElements()) { + if (auto *defaultArg = dyn_cast(element)) { + const ParamDecl *defaultParam = getParameterAt( + cast(defaultArg->getDefaultArgsOwner().getDecl()), + count++); + tupleExpr->setElement(count - 1, defaultParam->getDefaultValue()); + } else { + (void)++count; + } + } + } + // Evaluate the index arguments. SmallVector indexValues; auto indexResult = visit(component.getIndexExpr(), SGFContext()); diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index cfd9b5257bff7..9d956b8b0747e 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -322,6 +322,25 @@ struct Subscripts { } } +struct SubscriptDefaults { + 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() } + } +} + // CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[] @@ -352,6 +371,16 @@ func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[Bass()] _ = \Subscripts.[Treble()] + + _ = \SubscriptDefaults.[] + _ = \SubscriptDefaults.[0] + _ = \SubscriptDefaults.[0, 0] + _ = \SubscriptDefaults.[0, 0, 0] + + _ = \SubscriptDefaults.[false] + _ = \SubscriptDefaults.[false, bool: false] + _ = \SubscriptDefaults.[bool: false, 0] + _ = \SubscriptDefaults.[bool: false, 0, 0] } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From 0376dd01abbbead81c4b36b6895a211e5740b239 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 30 Oct 2019 17:58:46 -0700 Subject: [PATCH 02/29] Refactor as suggested by @xedin --- lib/SILGen/SILGenExpr.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 3bb067e40aeef..378b7b9e3cdaf 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3560,15 +3560,12 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { } if (auto *tupleExpr = dyn_cast(component.getIndexExpr())) { - size_t count = 0; - for (auto *element : tupleExpr->getElements()) { - if (auto *defaultArg = dyn_cast(element)) { + for (size_t i = 0, n = tupleExpr->getNumElements(); i < n; ++i) { + if (auto *defaultArg = dyn_cast(tupleExpr->getElement(i))) { const ParamDecl *defaultParam = getParameterAt( cast(defaultArg->getDefaultArgsOwner().getDecl()), - count++); - tupleExpr->setElement(count - 1, defaultParam->getDefaultValue()); - } else { - (void)++count; + i); + tupleExpr->setElement(i, defaultParam->getDefaultValue()); } } } From 4da9c425db168fe6bf2c79d72684972367ea543a Mon Sep 17 00:00:00 2001 From: zoecarver Date: Thu, 31 Oct 2019 11:03:24 -0700 Subject: [PATCH 03/29] Stash progress before changing branches --- lib/AST/ASTWalker.cpp | 4 ++++ lib/SILGen/SILGenExpr.cpp | 20 ------------------ lib/Sema/CSApply.cpp | 44 ++++++++++++++++++++++++++++++++++++++- lib/Sema/CSGen.cpp | 3 ++- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index 1bd64abdd41e7..2d72eae6c379e 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -552,6 +552,8 @@ class Traversal : public ASTVisitordump(); + if (Expr *subExpr = doIt(E->getSubExpr())) { E->setSubExpr(subExpr); return E; @@ -1011,6 +1013,8 @@ class Traversal : public ASTVisitordump(); + // For an ObjC key path, the string literal expr serves as the semantic // expression. if (auto objcStringLiteral = E->getObjCStringLiteralExpr()) { diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 378b7b9e3cdaf..61559ddd50715 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3550,26 +3550,6 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { if (!component.getIndexExpr()) return; - if (auto *parenExpr = dyn_cast(component.getIndexExpr())) { - if (auto *defaultArg = - dyn_cast(parenExpr->getSubExpr())) { - const ParamDecl *defaultParam = getParameterAt( - cast(defaultArg->getDefaultArgsOwner().getDecl()), 0); - parenExpr->setSubExpr(defaultParam->getDefaultValue()); - } - } - - if (auto *tupleExpr = dyn_cast(component.getIndexExpr())) { - for (size_t i = 0, n = tupleExpr->getNumElements(); i < n; ++i) { - if (auto *defaultArg = dyn_cast(tupleExpr->getElement(i))) { - const ParamDecl *defaultParam = getParameterAt( - cast(defaultArg->getDefaultArgsOwner().getDecl()), - i); - tupleExpr->setElement(i, defaultParam->getDefaultValue()); - } - } - } - // Evaluate the index arguments. SmallVector indexValues; auto indexResult = visit(component.getIndexExpr(), SGFContext()); diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index d8330e70bbdee..ff8db0e9b82f0 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4201,7 +4201,7 @@ namespace { return E; } - Expr *visitKeyPathExpr(KeyPathExpr *E) { + Expr *visitKeyPathExpr(KeyPathExpr *E) { if (E->isObjC()) { cs.setType(E, cs.getType(E->getObjCStringLiteralExpr())); return E; @@ -4656,6 +4656,27 @@ namespace { conformances.push_back(*hashableConformance); } + + newIndexExpr->dump(); + if (auto *parenExpr = dyn_cast(newIndexExpr)) { + if (auto *defaultArg = + dyn_cast(parenExpr->getSubExpr())) { + const ParamDecl *defaultParam = getParameterAt( + cast(defaultArg->getDefaultArgsOwner().getDecl()), 0); + parenExpr->setSubExpr(defaultParam->getDefaultValue()); + } + } + + if (auto *tupleExpr = dyn_cast(newIndexExpr)) { + for (size_t i = 0, n = tupleExpr->getNumElements(); i < n; ++i) { + if (auto *defaultArg = dyn_cast(tupleExpr->getElement(i))) { + const ParamDecl *defaultParam = getParameterAt( + cast(defaultArg->getDefaultArgsOwner().getDecl()), + i); + tupleExpr->setElement(i, defaultParam->getDefaultValue()); + } + } + } component.setSubscriptIndexHashableConformances(conformances); return component; @@ -5533,6 +5554,27 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType, } } +// arg->dump(); +// if (auto *parenExpr = dyn_cast(arg)) { +// if (auto *defaultArg = +// dyn_cast(parenExpr->getSubExpr())) { +// const ParamDecl *defaultParam = getParameterAt( +// cast(defaultArg->getDefaultArgsOwner().getDecl()), 0); +// parenExpr->setSubExpr(defaultParam->getDefaultValue()); +// } +// } +// +// if (auto *tupleExpr = dyn_cast(arg)) { +// for (size_t i = 0, n = tupleExpr->getNumElements(); i < n; ++i) { +// if (auto *defaultArg = dyn_cast(tupleExpr->getElement(i))) { +// const ParamDecl *defaultParam = getParameterAt( +// cast(defaultArg->getDefaultArgsOwner().getDecl()), +// i); +// tupleExpr->setElement(i, defaultParam->getDefaultValue()); +// } +// } +// } + arg->setType(paramType); return cs.cacheType(arg); } diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index d3d4cb4f90296..8eb5eaaf390af 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1687,6 +1687,7 @@ namespace { } Type visitIdentityExpr(IdentityExpr *expr) { + expr->dump(); return CS.getType(expr->getSubExpr()); } @@ -1718,7 +1719,7 @@ namespace { return optTy; } - virtual Type visitParenExpr(ParenExpr *expr) { + virtual Type visitParenExpr(ParenExpr *expr) { if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) { CS.setFavoredType(expr, favoredTy); } From 14a2ad6ab8256a1116ecfa4637cd160e64bf1004 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 1 Nov 2019 09:25:47 -0700 Subject: [PATCH 04/29] Move default argument lowering into coerceCallArguments --- lib/Sema/CSApply.cpp | 55 ++++++++------------------------------------ 1 file changed, 9 insertions(+), 46 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index ff8db0e9b82f0..9de4d7f7628fb 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4656,27 +4656,6 @@ namespace { conformances.push_back(*hashableConformance); } - - newIndexExpr->dump(); - if (auto *parenExpr = dyn_cast(newIndexExpr)) { - if (auto *defaultArg = - dyn_cast(parenExpr->getSubExpr())) { - const ParamDecl *defaultParam = getParameterAt( - cast(defaultArg->getDefaultArgsOwner().getDecl()), 0); - parenExpr->setSubExpr(defaultParam->getDefaultValue()); - } - } - - if (auto *tupleExpr = dyn_cast(newIndexExpr)) { - for (size_t i = 0, n = tupleExpr->getNumElements(); i < n; ++i) { - if (auto *defaultArg = dyn_cast(tupleExpr->getElement(i))) { - const ParamDecl *defaultParam = getParameterAt( - cast(defaultArg->getDefaultArgsOwner().getDecl()), - i); - tupleExpr->setElement(i, defaultParam->getDefaultValue()); - } - } - } component.setSubscriptIndexHashableConformances(conformances); return component; @@ -5421,10 +5400,15 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType, // Otherwise, create a call of the default argument generator. } else { - defArg = - new (tc.Context) DefaultArgumentExpr(callee, paramIdx, - arg->getStartLoc(), - param.getParameterType()); + const ParamDecl *defaultParam = getParameterAt(cast(callee.getDecl()), newArgs.size()); + defArg = defaultParam->getDefaultValue(); + + if (defArg == nullptr) { + defArg = + new (tc.Context) DefaultArgumentExpr(callee, paramIdx, + arg->getStartLoc(), + param.getParameterType()); + } } cs.cacheType(defArg); @@ -5554,27 +5538,6 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType, } } -// arg->dump(); -// if (auto *parenExpr = dyn_cast(arg)) { -// if (auto *defaultArg = -// dyn_cast(parenExpr->getSubExpr())) { -// const ParamDecl *defaultParam = getParameterAt( -// cast(defaultArg->getDefaultArgsOwner().getDecl()), 0); -// parenExpr->setSubExpr(defaultParam->getDefaultValue()); -// } -// } -// -// if (auto *tupleExpr = dyn_cast(arg)) { -// for (size_t i = 0, n = tupleExpr->getNumElements(); i < n; ++i) { -// if (auto *defaultArg = dyn_cast(tupleExpr->getElement(i))) { -// const ParamDecl *defaultParam = getParameterAt( -// cast(defaultArg->getDefaultArgsOwner().getDecl()), -// i); -// tupleExpr->setElement(i, defaultParam->getDefaultValue()); -// } -// } -// } - arg->setType(paramType); return cs.cacheType(arg); } From ff29b8e9c47b43c1925bfa1f909759c32aa54140 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Fri, 1 Nov 2019 09:28:05 -0700 Subject: [PATCH 05/29] Clean up spacing and remove lingering debugging calls --- lib/AST/ASTWalker.cpp | 4 ---- lib/SILGen/SILGenExpr.cpp | 2 +- lib/Sema/CSApply.cpp | 4 ++-- lib/Sema/CSGen.cpp | 3 +-- 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index 2d72eae6c379e..1bd64abdd41e7 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -552,8 +552,6 @@ class Traversal : public ASTVisitordump(); - if (Expr *subExpr = doIt(E->getSubExpr())) { E->setSubExpr(subExpr); return E; @@ -1013,8 +1011,6 @@ class Traversal : public ASTVisitordump(); - // For an ObjC key path, the string literal expr serves as the semantic // expression. if (auto objcStringLiteral = E->getObjCStringLiteralExpr()) { diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 61559ddd50715..30ccec9ef7cf4 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3549,7 +3549,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { [this, &operands, E](const KeyPathExpr::Component &component) { if (!component.getIndexExpr()) return; - + // Evaluate the index arguments. SmallVector indexValues; auto indexResult = visit(component.getIndexExpr(), SGFContext()); diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 9de4d7f7628fb..fe700cb2b6513 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4201,7 +4201,7 @@ namespace { return E; } - Expr *visitKeyPathExpr(KeyPathExpr *E) { + Expr *visitKeyPathExpr(KeyPathExpr *E) { if (E->isObjC()) { cs.setType(E, cs.getType(E->getObjCStringLiteralExpr())); return E; @@ -5402,7 +5402,7 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType, } else { const ParamDecl *defaultParam = getParameterAt(cast(callee.getDecl()), newArgs.size()); defArg = defaultParam->getDefaultValue(); - + if (defArg == nullptr) { defArg = new (tc.Context) DefaultArgumentExpr(callee, paramIdx, diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 8eb5eaaf390af..d3d4cb4f90296 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1687,7 +1687,6 @@ namespace { } Type visitIdentityExpr(IdentityExpr *expr) { - expr->dump(); return CS.getType(expr->getSubExpr()); } @@ -1719,7 +1718,7 @@ namespace { return optTy; } - virtual Type visitParenExpr(ParenExpr *expr) { + virtual Type visitParenExpr(ParenExpr *expr) { if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) { CS.setFavoredType(expr, favoredTy); } From 327c08a983c029ff576949a792b4576c3f9640ff Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 6 Nov 2019 09:30:01 -0800 Subject: [PATCH 06/29] Add edgecases to tests --- test/SILGen/keypaths.swift | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index 9d956b8b0747e..bea888ba744f8 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -322,7 +322,7 @@ struct Subscripts { } } -struct SubscriptDefaults { +struct SubscriptDefaults1 { subscript(x: Int = 0) -> Int { get { fatalError() } set { fatalError() } @@ -341,6 +341,20 @@ struct SubscriptDefaults { } } +struct SubscriptDefaults2 { + subscript(x: Int? = nil) -> Int { + get { fatalError() } + set { fatalError() } + } +} + +struct SubscriptDefaults3 { + subscript(x: Int = #line) -> Int { + get { fatalError() } + set { fatalError() } + } +} + // CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[] @@ -372,15 +386,18 @@ func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[Bass()] _ = \Subscripts.[Treble()] - _ = \SubscriptDefaults.[] - _ = \SubscriptDefaults.[0] - _ = \SubscriptDefaults.[0, 0] - _ = \SubscriptDefaults.[0, 0, 0] + _ = \SubscriptDefaults1.[] + _ = \SubscriptDefaults1.[0] + _ = \SubscriptDefaults1.[0, 0] + _ = \SubscriptDefaults1.[0, 0, 0] - _ = \SubscriptDefaults.[false] - _ = \SubscriptDefaults.[false, bool: false] - _ = \SubscriptDefaults.[bool: false, 0] - _ = \SubscriptDefaults.[bool: false, 0, 0] + _ = \SubscriptDefaults1.[false] + _ = \SubscriptDefaults1.[false, bool: false] + _ = \SubscriptDefaults1.[bool: false, 0] + _ = \SubscriptDefaults1.[bool: false, 0, 0] + + _ = \SubscriptDefaults2.[] + _ = \SubscriptDefaults3.[] } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From 00ebbee3416e6f72f1c6feae293a1437ec8eb07e Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 6 Nov 2019 11:02:57 -0800 Subject: [PATCH 07/29] Fix based on review comments --- lib/Sema/CSApply.cpp | 3 ++- test/SILGen/keypaths.swift | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index c4baa20e0ca6b..ce62c8706a700 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -5399,7 +5399,8 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType, // Otherwise, create a call of the default argument generator. } else { - const ParamDecl *defaultParam = getParameterAt(cast(callee.getDecl()), newArgs.size()); + const ParamDecl *defaultParam = getParameterAt(callee.getDecl(), + newArgs.size()); defArg = defaultParam->getDefaultValue(); if (defArg == nullptr) { diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index bea888ba744f8..2c4cf966dce2d 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -397,7 +397,10 @@ func subscripts(x: T, y: U, s: String) { _ = \SubscriptDefaults1.[bool: false, 0, 0] _ = \SubscriptDefaults2.[] + _ = \SubscriptDefaults2.[0] + _ = \SubscriptDefaults3.[] + _ = \SubscriptDefaults3.[0] } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From 3da279933fcaf9cb14acdd83ff15c215912356a5 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sat, 9 Nov 2019 22:24:54 -0800 Subject: [PATCH 08/29] Emit default arguments in silgen --- lib/SILGen/SILGen.cpp | 2 +- lib/SILGen/SILGenExpr.cpp | 3 ++- lib/Sema/CSApply.cpp | 14 ++++---------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index fb8031d865bad..0e414e2532ec6 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1064,7 +1064,7 @@ void SILGenModule::emitDefaultArgGenerator(SILDeclRef constant, auto arg = param->getDefaultValue(); emitOrDelayFunction(*this, constant, [this,constant,arg,initDC](SILFunction *f) { - preEmitFunction(constant, arg, f, arg); +// preEmitFunction(constant, arg, f, arg); PrettyStackTraceSILFunction X("silgen emitDefaultArgGenerator ", f); SILGenFunction SGF(*this, *f, initDC); SGF.emitGeneratorFunction(constant, arg); diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 65f4e81ffc9a7..2721ddece9c18 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3574,12 +3574,13 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { } }; - for (auto &component : E->getComponents()) { switch (auto kind = component.getKind()) { case KeyPathExpr::Component::Kind::Property: case KeyPathExpr::Component::Kind::Subscript: { auto decl = cast(component.getDeclRef().getDecl()); + SGF.SGM.emitDefaultArgGenerators(dyn_cast(decl), + dyn_cast(decl)->getIndices()); unsigned numOperands = operands.size(); loweredComponents.push_back( diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index ce62c8706a700..2f6b10a389c7e 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -5399,16 +5399,10 @@ Expr *ExprRewriter::coerceCallArguments(Expr *arg, AnyFunctionType *funcType, // Otherwise, create a call of the default argument generator. } else { - const ParamDecl *defaultParam = getParameterAt(callee.getDecl(), - newArgs.size()); - defArg = defaultParam->getDefaultValue(); - - if (defArg == nullptr) { - defArg = - new (tc.Context) DefaultArgumentExpr(callee, paramIdx, - arg->getStartLoc(), - param.getParameterType()); - } + defArg = + new (tc.Context) DefaultArgumentExpr(callee, paramIdx, + arg->getStartLoc(), + param.getParameterType()); } cs.cacheType(defArg); From 8c3bfc598bf6bea789987afb04dd5aaa1fc675f9 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 10 Nov 2019 15:47:34 -0800 Subject: [PATCH 09/29] Move default arg generation into SILGen --- include/swift/AST/Expr.h | 20 ++++++++ lib/SILGen/SILGen.cpp | 2 +- lib/SILGen/SILGenExpr.cpp | 79 ++++++++++++++++++++++++++++++-- test/SILGen/keypaths.swift | 94 +++++++++++++++++++------------------- 4 files changed, 144 insertions(+), 51 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index a0267c8782554..74c45e5f8c4e9 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -5119,6 +5119,26 @@ class KeyPathExpr : public Expr { } llvm_unreachable("unhandled kind"); } + + void setIndexExpr(Expr *newExpr) { + switch (getKind()) { + case Kind::Subscript: + case Kind::UnresolvedSubscript: + SubscriptIndexExpr = newExpr; + return; + + case Kind::Invalid: + case Kind::OptionalChain: + case Kind::OptionalWrap: + case Kind::OptionalForce: + case Kind::UnresolvedProperty: + case Kind::Property: + case Kind::Identity: + case Kind::TupleElement: + return; + } + llvm_unreachable("unhandled kind"); + } ArrayRef getSubscriptLabels() const { switch (getKind()) { diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index fe2b7b19b06df..b1e5e9c22937f 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1063,7 +1063,7 @@ void SILGenModule::emitDefaultArgGenerator(SILDeclRef constant, auto arg = param->getDefaultValue(); emitOrDelayFunction(*this, constant, [this,constant,arg,initDC](SILFunction *f) { -// preEmitFunction(constant, arg, f, arg); + preEmitFunction(constant, arg, f, arg); PrettyStackTraceSILFunction X("silgen emitDefaultArgGenerator ", f); SILGenFunction SGF(*this, *f, initDC); SGF.emitGeneratorFunction(constant, arg); diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 2721ddece9c18..cf16a64667f29 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -50,6 +50,8 @@ #include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/raw_ostream.h" +#include + #include "swift/AST/DiagnosticsSIL.h" using namespace swift; @@ -460,6 +462,8 @@ namespace { RValue visitEditorPlaceholderExpr(EditorPlaceholderExpr *E, SGFContext C); RValue visitObjCSelectorExpr(ObjCSelectorExpr *E, SGFContext C); RValue visitKeyPathExpr(KeyPathExpr *E, SGFContext C); + void generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, + KeyPathExpr::Component &component); RValue visitMagicIdentifierLiteralExpr(MagicIdentifierLiteralExpr *E, SGFContext C); RValue visitCollectionExpr(CollectionExpr *E, SGFContext C); @@ -3532,6 +3536,74 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc, llvm_unreachable("unknown kind of storage"); } +// There is some uglyness here because this function has to not only handle +// the generation of default arguments but it also has to create a new TupleExpr +// or ParenExpr depending on how many default args are found. +void RValueEmitter::generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, + KeyPathExpr::Component &component) { + // Collect both all the args (default and not) into one place and + // keep track of their types. + llvm::SmallVector newTypeEls; + newTypeEls.reserve(S->getIndices()->size()); + llvm::SmallVector newArgs; + newArgs.reserve(S->getIndices()->size()); + size_t index = 0; + for (auto param : *S->getIndices()) { + newTypeEls.push_back(TupleTypeElt(param->getType())); + + if (param->isDefaultArgument()) { + newArgs.push_back(param->getDefaultValue()); + } else { + if (auto paren = dyn_cast(component.getIndexExpr())) { + newArgs.push_back(paren->getSubExpr()); + } else if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { + newArgs.push_back(tupleExpr->getElement(index)); + } + } + + (void)++index; + } + + // This looks it's going to give us a TypleType but it will give us a + // ParenType when newTypeEls.size() == 1. + auto newIndexType = TupleType::get(newTypeEls, SGF.getASTContext()); + + // Unconditionally recreate the index expr based on the new type. + if (isa(newIndexType.getPointer())) { + assert(newArgs.size() == 1 && + "If this was converted to a paren expr it should have exact one element"); + auto newIndexExpr = new (SGF.getASTContext()) + ParenExpr(component.getIndexExpr()->getStartLoc(), + newArgs[0], + component.getIndexExpr()->getEndLoc(), + /*hasTrailingClosure=*/false); + + component.setIndexExpr(newIndexExpr); + } else if (isa(newIndexType.getPointer())) { + SmallVector elementNames; + elementNames.reserve(newArgs.size()); + if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { + std::copy(tupleExpr->getElementNames().begin(), + tupleExpr->getElementNames().begin(), + elementNames.begin()); + } else { + std::transform(newArgs.begin(), newArgs.end(), + elementNames.begin(), + [](auto&&) { + return Identifier(); + }); + } + + auto newIndexExpr = TupleExpr::createImplicit(SGF.getASTContext(), + newArgs, elementNames); + component.setIndexExpr(newIndexExpr); + } else { + llvm_unreachable("Created invalid type"); + } + + component.getIndexExpr()->setType(newIndexType); +} + RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { if (E->isObjC()) { return visit(E->getObjCStringLiteralExpr(), C); @@ -3574,13 +3646,14 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { } }; - for (auto &component : E->getComponents()) { + for (auto &component : E->getMutableComponents()) { switch (auto kind = component.getKind()) { case KeyPathExpr::Component::Kind::Property: case KeyPathExpr::Component::Kind::Subscript: { auto decl = cast(component.getDeclRef().getDecl()); - SGF.SGM.emitDefaultArgGenerators(dyn_cast(decl), - dyn_cast(decl)->getIndices()); + if (auto subscriptDecl = dyn_cast(decl)) { + generateDefaultArgsForKeyPathSubscript(subscriptDecl, component); + } unsigned numOperands = operands.size(); loweredComponents.push_back( diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index 2c4cf966dce2d..eabb2c9562798 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -322,38 +322,38 @@ struct Subscripts { } } -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 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() } +// } +//} // CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts func subscripts(x: T, y: U, s: String) { @@ -386,21 +386,21 @@ func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[Bass()] _ = \Subscripts.[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] +// _ = \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] } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From f7dd8138c2b8dc45026c7cc0cd99816773aa7d5f Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 10 Nov 2019 15:49:27 -0800 Subject: [PATCH 10/29] Run code through clang-format --- include/swift/AST/Expr.h | 2 +- lib/SILGen/SILGenExpr.cpp | 53 ++++++++++++++++++--------------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 74c45e5f8c4e9..eab6082c1b05d 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -5119,7 +5119,7 @@ class KeyPathExpr : public Expr { } llvm_unreachable("unhandled kind"); } - + void setIndexExpr(Expr *newExpr) { switch (getKind()) { case Kind::Subscript: diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index cf16a64667f29..e01126cb94e55 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -462,8 +462,9 @@ namespace { RValue visitEditorPlaceholderExpr(EditorPlaceholderExpr *E, SGFContext C); RValue visitObjCSelectorExpr(ObjCSelectorExpr *E, SGFContext C); RValue visitKeyPathExpr(KeyPathExpr *E, SGFContext C); - void generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, - KeyPathExpr::Component &component); + void + generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, + KeyPathExpr::Component &component); RValue visitMagicIdentifierLiteralExpr(MagicIdentifierLiteralExpr *E, SGFContext C); RValue visitCollectionExpr(CollectionExpr *E, SGFContext C); @@ -3539,44 +3540,44 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc, // There is some uglyness here because this function has to not only handle // the generation of default arguments but it also has to create a new TupleExpr // or ParenExpr depending on how many default args are found. -void RValueEmitter::generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, - KeyPathExpr::Component &component) { +void RValueEmitter::generateDefaultArgsForKeyPathSubscript( + SubscriptDecl *S, KeyPathExpr::Component &component) { // Collect both all the args (default and not) into one place and // keep track of their types. llvm::SmallVector newTypeEls; newTypeEls.reserve(S->getIndices()->size()); - llvm::SmallVector newArgs; + llvm::SmallVector newArgs; newArgs.reserve(S->getIndices()->size()); size_t index = 0; for (auto param : *S->getIndices()) { newTypeEls.push_back(TupleTypeElt(param->getType())); - + if (param->isDefaultArgument()) { newArgs.push_back(param->getDefaultValue()); } else { if (auto paren = dyn_cast(component.getIndexExpr())) { newArgs.push_back(paren->getSubExpr()); - } else if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { + } else if (auto tupleExpr = + dyn_cast(component.getIndexExpr())) { newArgs.push_back(tupleExpr->getElement(index)); } } (void)++index; } - + // This looks it's going to give us a TypleType but it will give us a // ParenType when newTypeEls.size() == 1. auto newIndexType = TupleType::get(newTypeEls, SGF.getASTContext()); - + // Unconditionally recreate the index expr based on the new type. if (isa(newIndexType.getPointer())) { - assert(newArgs.size() == 1 && - "If this was converted to a paren expr it should have exact one element"); + assert(newArgs.size() == 1 && "If this was converted to a paren expr it " + "should have exact one element"); auto newIndexExpr = new (SGF.getASTContext()) - ParenExpr(component.getIndexExpr()->getStartLoc(), - newArgs[0], - component.getIndexExpr()->getEndLoc(), - /*hasTrailingClosure=*/false); + ParenExpr(component.getIndexExpr()->getStartLoc(), newArgs[0], + component.getIndexExpr()->getEndLoc(), + /*hasTrailingClosure=*/false); component.setIndexExpr(newIndexExpr); } else if (isa(newIndexType.getPointer())) { @@ -3584,23 +3585,19 @@ void RValueEmitter::generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, elementNames.reserve(newArgs.size()); if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { std::copy(tupleExpr->getElementNames().begin(), - tupleExpr->getElementNames().begin(), - elementNames.begin()); + tupleExpr->getElementNames().begin(), elementNames.begin()); } else { - std::transform(newArgs.begin(), newArgs.end(), - elementNames.begin(), - [](auto&&) { - return Identifier(); - }); + std::transform(newArgs.begin(), newArgs.end(), elementNames.begin(), + [](auto &&) { return Identifier(); }); } - - auto newIndexExpr = TupleExpr::createImplicit(SGF.getASTContext(), - newArgs, elementNames); + + auto newIndexExpr = + TupleExpr::createImplicit(SGF.getASTContext(), newArgs, elementNames); component.setIndexExpr(newIndexExpr); } else { llvm_unreachable("Created invalid type"); } - + component.getIndexExpr()->setType(newIndexType); } @@ -3645,7 +3642,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { std::move(rv).forwardAsSingleValue(SGF, E)); } }; - + for (auto &component : E->getMutableComponents()) { switch (auto kind = component.getKind()) { case KeyPathExpr::Component::Kind::Property: @@ -3726,7 +3723,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { llvm_unreachable("not resolved"); } } - + StringRef objcString; if (auto objcExpr = dyn_cast_or_null (E->getObjCStringLiteralExpr())) From b6bca529ec795ac093cae7363c77bfd774ab6f9b Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 10 Nov 2019 15:58:55 -0800 Subject: [PATCH 11/29] Remove lingering newline --- lib/SILGen/SILGenExpr.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index e01126cb94e55..2d3f5eba34fbf 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3723,7 +3723,6 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { llvm_unreachable("not resolved"); } } - StringRef objcString; if (auto objcExpr = dyn_cast_or_null (E->getObjCStringLiteralExpr())) From b0877fb4d1ee233d0645be9aa97ea2f73bd34ee4 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 10 Nov 2019 15:59:38 -0800 Subject: [PATCH 12/29] Re-enable tests (oops) --- test/SILGen/keypaths.swift | 94 +++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index eabb2c9562798..2c4cf966dce2d 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -322,38 +322,38 @@ struct Subscripts { } } -//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 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() } + } +} // CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts func subscripts(x: T, y: U, s: String) { @@ -386,21 +386,21 @@ func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[Bass()] _ = \Subscripts.[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] + _ = \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] } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From 921c1ebfe91e04a113e0ae814f5335a204a74ec2 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 10 Nov 2019 17:54:02 -0800 Subject: [PATCH 13/29] Move default arg generation into lambda instead of modifing AST --- include/swift/AST/Expr.h | 20 ----- lib/SILGen/SILGenExpr.cpp | 150 +++++++++++++++----------------------- 2 files changed, 58 insertions(+), 112 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index eab6082c1b05d..a0267c8782554 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -5120,26 +5120,6 @@ class KeyPathExpr : public Expr { llvm_unreachable("unhandled kind"); } - void setIndexExpr(Expr *newExpr) { - switch (getKind()) { - case Kind::Subscript: - case Kind::UnresolvedSubscript: - SubscriptIndexExpr = newExpr; - return; - - case Kind::Invalid: - case Kind::OptionalChain: - case Kind::OptionalWrap: - case Kind::OptionalForce: - case Kind::UnresolvedProperty: - case Kind::Property: - case Kind::Identity: - case Kind::TupleElement: - return; - } - llvm_unreachable("unhandled kind"); - } - ArrayRef getSubscriptLabels() const { switch (getKind()) { case Kind::Subscript: diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 2d3f5eba34fbf..322d17c2bf9c8 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -50,8 +50,6 @@ #include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/raw_ostream.h" -#include - #include "swift/AST/DiagnosticsSIL.h" using namespace swift; @@ -462,9 +460,6 @@ namespace { RValue visitEditorPlaceholderExpr(EditorPlaceholderExpr *E, SGFContext C); RValue visitObjCSelectorExpr(ObjCSelectorExpr *E, SGFContext C); RValue visitKeyPathExpr(KeyPathExpr *E, SGFContext C); - void - generateDefaultArgsForKeyPathSubscript(SubscriptDecl *S, - KeyPathExpr::Component &component); RValue visitMagicIdentifierLiteralExpr(MagicIdentifierLiteralExpr *E, SGFContext C); RValue visitCollectionExpr(CollectionExpr *E, SGFContext C); @@ -3537,70 +3532,6 @@ SILGenModule::emitKeyPathComponentForDecl(SILLocation loc, llvm_unreachable("unknown kind of storage"); } -// There is some uglyness here because this function has to not only handle -// the generation of default arguments but it also has to create a new TupleExpr -// or ParenExpr depending on how many default args are found. -void RValueEmitter::generateDefaultArgsForKeyPathSubscript( - SubscriptDecl *S, KeyPathExpr::Component &component) { - // Collect both all the args (default and not) into one place and - // keep track of their types. - llvm::SmallVector newTypeEls; - newTypeEls.reserve(S->getIndices()->size()); - llvm::SmallVector newArgs; - newArgs.reserve(S->getIndices()->size()); - size_t index = 0; - for (auto param : *S->getIndices()) { - newTypeEls.push_back(TupleTypeElt(param->getType())); - - if (param->isDefaultArgument()) { - newArgs.push_back(param->getDefaultValue()); - } else { - if (auto paren = dyn_cast(component.getIndexExpr())) { - newArgs.push_back(paren->getSubExpr()); - } else if (auto tupleExpr = - dyn_cast(component.getIndexExpr())) { - newArgs.push_back(tupleExpr->getElement(index)); - } - } - - (void)++index; - } - - // This looks it's going to give us a TypleType but it will give us a - // ParenType when newTypeEls.size() == 1. - auto newIndexType = TupleType::get(newTypeEls, SGF.getASTContext()); - - // Unconditionally recreate the index expr based on the new type. - if (isa(newIndexType.getPointer())) { - assert(newArgs.size() == 1 && "If this was converted to a paren expr it " - "should have exact one element"); - auto newIndexExpr = new (SGF.getASTContext()) - ParenExpr(component.getIndexExpr()->getStartLoc(), newArgs[0], - component.getIndexExpr()->getEndLoc(), - /*hasTrailingClosure=*/false); - - component.setIndexExpr(newIndexExpr); - } else if (isa(newIndexType.getPointer())) { - SmallVector elementNames; - elementNames.reserve(newArgs.size()); - if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { - std::copy(tupleExpr->getElementNames().begin(), - tupleExpr->getElementNames().begin(), elementNames.begin()); - } else { - std::transform(newArgs.begin(), newArgs.end(), elementNames.begin(), - [](auto &&) { return Identifier(); }); - } - - auto newIndexExpr = - TupleExpr::createImplicit(SGF.getASTContext(), newArgs, elementNames); - component.setIndexExpr(newIndexExpr); - } else { - llvm_unreachable("Created invalid type"); - } - - component.getIndexExpr()->setType(newIndexType); -} - RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { if (E->isObjC()) { return visit(E->getObjCStringLiteralExpr(), C); @@ -3622,35 +3553,66 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { auto baseTy = rootTy; SmallVector operands; - - auto lowerSubscriptOperands = - [this, &operands, E](const KeyPathExpr::Component &component) { - if (!component.getIndexExpr()) - return; - - // Evaluate the index arguments. - SmallVector indexValues; - auto indexResult = visit(component.getIndexExpr(), SGFContext()); - if (isa(indexResult.getType())) { - std::move(indexResult).extractElements(indexValues); - } else { + + auto lowerSubscriptOperands = [this, &operands, + E](const KeyPathExpr::Component &component) { + if (!component.getIndexExpr()) + return; + + auto decl = dyn_cast(component.getDeclRef().getDecl()); + assert(decl && + "lowerSubscriptOperands must be called with a subscript decl"); + + // Evaluate the index arguments. + SmallVector indexValues; + RValue indexResult; + if (auto paren = dyn_cast(component.getIndexExpr())) { + auto param = decl->getIndices()->get(0); + if (param->isDefaultArgument()) + indexResult = visit(param->getDefaultValue(), SGFContext()); + else + indexResult = visit(paren, SGFContext()); + indexValues.push_back(std::move(indexResult)); + } else if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { + for (size_t index = 0; index < tupleExpr->getNumElements(); ++index) { + auto param = decl->getIndices()->get(index); + if (param->isDefaultArgument()) + indexResult = visit(param->getDefaultValue(), SGFContext()); + else + indexResult = visit(tupleExpr->getElement(index), SGFContext()); indexValues.push_back(std::move(indexResult)); } + } - for (auto &rv : indexValues) { - operands.push_back( - std::move(rv).forwardAsSingleValue(SGF, E)); - } - }; + for (auto &rv : indexValues) { + operands.push_back(std::move(rv).forwardAsSingleValue(SGF, E)); + } + }; + + auto lowerOperands = [this, &operands, + E](const KeyPathExpr::Component &component) { + if (!component.getIndexExpr()) + return; - for (auto &component : E->getMutableComponents()) { + // Evaluate the index arguments. + SmallVector indexValues; + auto indexResult = visit(component.getIndexExpr(), SGFContext()); + if (isa(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()) { case KeyPathExpr::Component::Kind::Property: case KeyPathExpr::Component::Kind::Subscript: { auto decl = cast(component.getDeclRef().getDecl()); - if (auto subscriptDecl = dyn_cast(decl)) { - generateDefaultArgsForKeyPathSubscript(subscriptDecl, component); - } unsigned numOperands = operands.size(); loweredComponents.push_back( @@ -3664,8 +3626,12 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { component.getSubscriptIndexHashableConformances(), baseTy, /*for descriptor*/ false)); - lowerSubscriptOperands(component); - + if (kind == KeyPathExpr::Component::Kind::Subscript) { + lowerSubscriptOperands(component); + } else { + lowerOperands(component); + } + assert(numOperands == operands.size() && "operand count out of sync"); baseTy = loweredComponents.back().getComponentType(); From 18f918b5dd630eee43c49dee8e2af13c24f99353 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 10 Nov 2019 20:19:34 -0800 Subject: [PATCH 14/29] Regenerate labels in CSApply instead of keeping two sizes --- include/swift/AST/Expr.h | 10 +++------- lib/AST/Expr.cpp | 4 ++-- lib/Sema/CSApply.cpp | 14 ++++++++++---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index a0267c8782554..7604f05d1592a 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -4915,10 +4915,7 @@ class KeyPathExpr : public Expr { const ProtocolConformanceRef *SubscriptHashableConformancesData; union { - struct { - unsigned numConformances; - unsigned numLabels; - } SubscriptSize; + unsigned SubscriptSize; unsigned TupleIndex; }; @@ -5124,7 +5121,7 @@ class KeyPathExpr : public Expr { switch (getKind()) { case Kind::Subscript: case Kind::UnresolvedSubscript: - return {SubscriptLabelsData, (size_t)SubscriptSize.numLabels}; + return {SubscriptLabelsData, (size_t)SubscriptSize}; case Kind::Invalid: case Kind::OptionalChain: @@ -5145,8 +5142,7 @@ class KeyPathExpr : public Expr { case Kind::Subscript: if (!SubscriptHashableConformancesData) return {}; - return {SubscriptHashableConformancesData, - (size_t)SubscriptSize.numConformances}; + return {SubscriptHashableConformancesData, (size_t)SubscriptSize}; case Kind::UnresolvedSubscript: case Kind::Invalid: diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index a72dbdd0fe8ef..edcdab2c38ffb 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2159,7 +2159,7 @@ KeyPathExpr::Component::Component(ASTContext *ctxForCopyingLabels, SubscriptLabelsData = subscriptLabels.data(); SubscriptHashableConformancesData = indexHashables.empty() ? nullptr : indexHashables.data(); - SubscriptSize.numLabels = subscriptLabels.size(); + SubscriptSize = subscriptLabels.size(); } KeyPathExpr::Component @@ -2176,7 +2176,7 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances( ArrayRef hashables) { switch (getKind()) { case Kind::Subscript: - SubscriptSize.numConformances = hashables.size(); + assert(hashables.size() == SubscriptSize); SubscriptHashableConformancesData = getComponentType()->getASTContext() .AllocateCopy(hashables) .data(); diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 187411d348b6e..52f4f73b43e79 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4590,9 +4590,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. @@ -4602,7 +4599,12 @@ namespace { cs.getASTContext().getProtocol(KnownProtocolKind::Hashable); auto fnType = overload.openedType->castTo(); - for (const auto ¶m : fnType->getParams()) { + auto params = fnType->getParams(); + SmallVector newLabels; + for (size_t index = 0; index < fnType->getNumParams(); ++index) { + auto param = params[index]; + 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 @@ -4615,6 +4617,10 @@ namespace { conformances.push_back(hashableConformance); } + auto component = + KeyPathExpr::Component::forSubscriptWithPrebuiltIndexExpr( + ref, newIndexExpr, newLabels, resolvedTy, componentLoc, {}); + component.setSubscriptIndexHashableConformances(conformances); return component; } From e385439e6efd254bae2f4295d5c80efad0613b5d Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 11 Nov 2019 16:10:48 -0800 Subject: [PATCH 15/29] Use ArgEmitter --- lib/SILGen/SILGenApply.cpp | 29 +++++++++++++++++++++++++++++ lib/SILGen/SILGenExpr.cpp | 22 ++++++++++++++-------- lib/SILGen/SILGenFunction.h | 3 +++ 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 00be4b4e930fa..a29bd3c2c51f9 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6067,6 +6067,35 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e, return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL)); } +void SILGenFunction::emitKeyPathSubscriptOperands(CanSILFunctionType fnType, + Expr *indexExpr) { + SmallVector argValues; + SmallVector delayedArgs; +// auto fnType = F.getLoweredFunctionType(); + ArgEmitter emitter(*this, fnType.getPointer()->getRepresentation(), /*yield*/ true, + /*isForCoroutine*/ false, + ClaimedParamsRef(fnType, fnType.getPointer()->getParameters()), + argValues, delayedArgs, + /*foreign error*/ None, ImportAsMemberStatus()); + + SmallVector indexExprs; + if (auto paren = dyn_cast(indexExpr)) { + indexExprs.push_back(indexExpr); + } else if (auto tupleExpr = dyn_cast(indexExpr)) { + for (auto *tupleElement : tupleExpr->getElements()) { + indexExprs.push_back(tupleElement); + } + } + + for (auto *expr : indexExprs) { + emitter.emitSingleArg(ArgumentSource(expr), + AbstractionPattern(expr->getType())); + } + + if (!delayedArgs.empty()) + emitDelayedArguments(*this, delayedArgs, argValues); +} + ManagedValue ArgumentScope::popPreservingValue(ManagedValue mv) { formalEvalScope.pop(); return normalScope.popPreservingValue(mv); diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 322d17c2bf9c8..919457ed78cbf 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3626,15 +3626,21 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { component.getSubscriptIndexHashableConformances(), baseTy, /*for descriptor*/ false)); - if (kind == KeyPathExpr::Component::Kind::Subscript) { - lowerSubscriptOperands(component); - } else { - lowerOperands(component); - } - - assert(numOperands == operands.size() - && "operand count out of sync"); +// if (kind == KeyPathExpr::Component::Kind::Subscript) { +// lowerSubscriptOperands(component); +// } else { +// lowerOperands(component); +// } + +// assert(numOperands == operands.size() +// && "operand count out of sync"); baseTy = loweredComponents.back().getComponentType(); + baseTy.dump(); +// loweredComponents.back().g + auto subscriptFn = loweredComponents.back().getComputedPropertyGetter(); + SGF.prepareSubscriptIndices(<#SubscriptDecl *subscript#>, <#SubstitutionMap subs#>, <#AccessStrategy strategy#>, <#Expr *indices#>) + SGF.emitKeyPathSubscriptOperands(SGF.F.getLoweredFunctionType(), // subscriptFn->getLoweredFunctionType(), + component.getIndexExpr()); break; } diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 0887461ea7992..ca6931e5313d7 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -665,6 +665,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction IsFreeFunctionWitness_t isFree, bool isSelfConformance); + void emitKeyPathSubscriptOperands(CanSILFunctionType fnType, + Expr *indexExpr); + /// Convert a block to a native function with a thunk. ManagedValue emitBlockToFunc(SILLocation loc, ManagedValue block, From 51dc7c85357010a11b42afa9e69f5c4ad01977e0 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 11 Nov 2019 19:35:47 -0800 Subject: [PATCH 16/29] Create emitKeyPathSubscriptOperands to lower all the args properly (including the default ones) --- lib/SILGen/SILGenApply.cpp | 48 ++++++++++++---------- lib/SILGen/SILGenExpr.cpp | 81 ++++++------------------------------- lib/SILGen/SILGenFunction.h | 9 +++-- 3 files changed, 44 insertions(+), 94 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index a29bd3c2c51f9..18d1ef321aba5 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6067,33 +6067,37 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e, return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL)); } -void SILGenFunction::emitKeyPathSubscriptOperands(CanSILFunctionType fnType, - Expr *indexExpr) { +SmallVector SILGenFunction::emitKeyPathSubscriptOperands( + SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr, + CanSILFunctionType fnType) { + Type interfaceType = subscript->getInterfaceType(); SmallVector argValues; SmallVector delayedArgs; -// auto fnType = F.getLoweredFunctionType(); - ArgEmitter emitter(*this, fnType.getPointer()->getRepresentation(), /*yield*/ true, - /*isForCoroutine*/ false, - ClaimedParamsRef(fnType, fnType.getPointer()->getParameters()), - argValues, delayedArgs, - /*foreign error*/ None, ImportAsMemberStatus()); - - SmallVector indexExprs; - if (auto paren = dyn_cast(indexExpr)) { - indexExprs.push_back(indexExpr); - } else if (auto tupleExpr = dyn_cast(indexExpr)) { - for (auto *tupleElement : tupleExpr->getElements()) { - indexExprs.push_back(tupleElement); - } - } - - for (auto *expr : indexExprs) { - emitter.emitSingleArg(ArgumentSource(expr), - AbstractionPattern(expr->getType())); - } + ArgEmitter emitter( + *this, fnType.getPointer()->getRepresentation(), + /*yield*/ true, + /*isForCoroutine*/ false, + ClaimedParamsRef(fnType, fnType.getPointer()->getParameters()), argValues, + delayedArgs, + /*foreign error*/ None, ImportAsMemberStatus()); + + CanFunctionType substFnType = + subs ? cast(interfaceType->castTo() + ->substGenericArgs(subs) + ->getCanonicalType()) + : cast(interfaceType->getCanonicalType()); + + AbstractionPattern origFnType(substFnType); + 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) { diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 919457ed78cbf..cf39e1cf832b5 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3554,60 +3554,6 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { auto baseTy = rootTy; SmallVector operands; - auto lowerSubscriptOperands = [this, &operands, - E](const KeyPathExpr::Component &component) { - if (!component.getIndexExpr()) - return; - - auto decl = dyn_cast(component.getDeclRef().getDecl()); - assert(decl && - "lowerSubscriptOperands must be called with a subscript decl"); - - // Evaluate the index arguments. - SmallVector indexValues; - RValue indexResult; - if (auto paren = dyn_cast(component.getIndexExpr())) { - auto param = decl->getIndices()->get(0); - if (param->isDefaultArgument()) - indexResult = visit(param->getDefaultValue(), SGFContext()); - else - indexResult = visit(paren, SGFContext()); - indexValues.push_back(std::move(indexResult)); - } else if (auto tupleExpr = dyn_cast(component.getIndexExpr())) { - for (size_t index = 0; index < tupleExpr->getNumElements(); ++index) { - auto param = decl->getIndices()->get(index); - if (param->isDefaultArgument()) - indexResult = visit(param->getDefaultValue(), SGFContext()); - else - indexResult = visit(tupleExpr->getElement(index), SGFContext()); - indexValues.push_back(std::move(indexResult)); - } - } - - for (auto &rv : indexValues) { - operands.push_back(std::move(rv).forwardAsSingleValue(SGF, E)); - } - }; - - auto lowerOperands = [this, &operands, - E](const KeyPathExpr::Component &component) { - if (!component.getIndexExpr()) - return; - - // Evaluate the index arguments. - SmallVector indexValues; - auto indexResult = visit(component.getIndexExpr(), SGFContext()); - if (isa(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()) { case KeyPathExpr::Component::Kind::Property: @@ -3626,21 +3572,20 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { component.getSubscriptIndexHashableConformances(), baseTy, /*for descriptor*/ false)); -// if (kind == KeyPathExpr::Component::Kind::Subscript) { -// lowerSubscriptOperands(component); -// } else { -// lowerOperands(component); -// } - -// assert(numOperands == operands.size() -// && "operand count out of sync"); baseTy = loweredComponents.back().getComponentType(); - baseTy.dump(); -// loweredComponents.back().g - auto subscriptFn = loweredComponents.back().getComputedPropertyGetter(); - SGF.prepareSubscriptIndices(<#SubscriptDecl *subscript#>, <#SubstitutionMap subs#>, <#AccessStrategy strategy#>, <#Expr *indices#>) - SGF.emitKeyPathSubscriptOperands(SGF.F.getLoweredFunctionType(), // subscriptFn->getLoweredFunctionType(), - component.getIndexExpr()); + if (kind == KeyPathExpr::Component::Kind::Property) + break; + + auto subscriptFn = + loweredComponents.back().getComputedPropertyId().getFunction(); + auto subscript = cast(decl); + auto loweredArgs = SGF.emitKeyPathSubscriptOperands( + subscript, component.getDeclRef().getSubstitutions(), + component.getIndexExpr(), subscriptFn->getLoweredFunctionType()); + + for (auto &arg : loweredArgs) { + operands.push_back(arg.forward(SGF)); + } break; } diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index ca6931e5313d7..480d69cfc59be 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -664,10 +664,11 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction SubstitutionMap witnessSubs, IsFreeFunctionWitness_t isFree, bool isSelfConformance); - - void emitKeyPathSubscriptOperands(CanSILFunctionType fnType, - Expr *indexExpr); - + + SmallVector + emitKeyPathSubscriptOperands(SubscriptDecl *subscript, SubstitutionMap subs, + Expr *indexExpr, CanSILFunctionType fnType); + /// Convert a block to a native function with a thunk. ManagedValue emitBlockToFunc(SILLocation loc, ManagedValue block, From 27ffff100e1cc0381b871450f338015fcc326c01 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 11 Nov 2019 19:37:16 -0800 Subject: [PATCH 17/29] Use range-based for-loop --- lib/Sema/CSApply.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 52f4f73b43e79..24d33e1599be7 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4599,10 +4599,8 @@ namespace { cs.getASTContext().getProtocol(KnownProtocolKind::Hashable); auto fnType = overload.openedType->castTo(); - auto params = fnType->getParams(); SmallVector newLabels; - for (size_t index = 0; index < fnType->getNumParams(); ++index) { - auto param = params[index]; + for (auto ¶m : fnType->getParams()) { newLabels.push_back(param.getLabel()); auto indexType = simplifyType(param.getPlainType()); From 0f3dd52972e3e4975af94c3dbb97610ab9296e8d Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 13:23:31 -0800 Subject: [PATCH 18/29] Use correct function type to prevent conflicting argument types --- lib/SILGen/SILGenApply.cpp | 21 ++++++++++----------- lib/SILGen/SILGenExpr.cpp | 4 +--- lib/SILGen/SILGenFunction.h | 2 +- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 18d1ef321aba5..23ddd4592d2b5 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6068,26 +6068,25 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e, } SmallVector SILGenFunction::emitKeyPathSubscriptOperands( - SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr, - CanSILFunctionType fnType) { + SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr) { Type interfaceType = subscript->getInterfaceType(); + CanFunctionType substFnType = + subs ? cast(interfaceType->castTo() + ->substGenericArgs(subs) + ->getCanonicalType()) + : cast(interfaceType->getCanonicalType()); + AbstractionPattern origFnType(substFnType); + auto fnType = getLoweredType(origFnType, substFnType).castTo(); SmallVector argValues; SmallVector delayedArgs; ArgEmitter emitter( *this, fnType.getPointer()->getRepresentation(), - /*yield*/ true, + /*yield*/ false, /*isForCoroutine*/ false, ClaimedParamsRef(fnType, fnType.getPointer()->getParameters()), argValues, delayedArgs, /*foreign error*/ None, ImportAsMemberStatus()); - - CanFunctionType substFnType = - subs ? cast(interfaceType->castTo() - ->substGenericArgs(subs) - ->getCanonicalType()) - : cast(interfaceType->getCanonicalType()); - - AbstractionPattern origFnType(substFnType); + auto prepared = prepareSubscriptIndices(subscript, subs, // Strategy doesn't matter diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index cf39e1cf832b5..ada5eba9ab35e 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3576,12 +3576,10 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { if (kind == KeyPathExpr::Component::Kind::Property) break; - auto subscriptFn = - loweredComponents.back().getComputedPropertyId().getFunction(); auto subscript = cast(decl); auto loweredArgs = SGF.emitKeyPathSubscriptOperands( subscript, component.getDeclRef().getSubstitutions(), - component.getIndexExpr(), subscriptFn->getLoweredFunctionType()); + component.getIndexExpr()); for (auto &arg : loweredArgs) { operands.push_back(arg.forward(SGF)); diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 480d69cfc59be..a0fdcc3bb40d3 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -667,7 +667,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction SmallVector emitKeyPathSubscriptOperands(SubscriptDecl *subscript, SubstitutionMap subs, - Expr *indexExpr, CanSILFunctionType fnType); + Expr *indexExpr); /// Convert a block to a native function with a thunk. ManagedValue emitBlockToFunc(SILLocation loc, From 08244697d11f760deea9f13aeba0d83b119ee3c2 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 13:24:49 -0800 Subject: [PATCH 19/29] Format properly --- lib/SILGen/SILGenApply.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 23ddd4592d2b5..fcb1c5e8721d7 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6076,7 +6076,8 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( ->getCanonicalType()) : cast(interfaceType->getCanonicalType()); AbstractionPattern origFnType(substFnType); - auto fnType = getLoweredType(origFnType, substFnType).castTo(); + auto fnType = + getLoweredType(origFnType, substFnType).castTo(); SmallVector argValues; SmallVector delayedArgs; ArgEmitter emitter( @@ -6086,7 +6087,7 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( ClaimedParamsRef(fnType, fnType.getPointer()->getParameters()), argValues, delayedArgs, /*foreign error*/ None, ImportAsMemberStatus()); - + auto prepared = prepareSubscriptIndices(subscript, subs, // Strategy doesn't matter From 853dfbefa0e6c539b300497a08e542bbca314d9b Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 13:34:01 -0800 Subject: [PATCH 20/29] Copy labels and conformances --- lib/Sema/CSApply.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 24d33e1599be7..ac319578edb25 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4615,12 +4615,10 @@ namespace { conformances.push_back(hashableConformance); } - auto component = - KeyPathExpr::Component::forSubscriptWithPrebuiltIndexExpr( - ref, newIndexExpr, newLabels, resolvedTy, componentLoc, {}); - - 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) { From 5e1e9075b2f4e5323cae72d5981b9981c2c35d2b Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 13:35:44 -0800 Subject: [PATCH 21/29] Fix spacing --- lib/SILGen/SILGenApply.cpp | 1 + lib/SILGen/SILGenExpr.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index fcb1c5e8721d7..2a004c70d69be 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6078,6 +6078,7 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( AbstractionPattern origFnType(substFnType); auto fnType = getLoweredType(origFnType, substFnType).castTo(); + SmallVector argValues; SmallVector delayedArgs; ArgEmitter emitter( diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index ada5eba9ab35e..7b4038d406e26 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3638,6 +3638,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { llvm_unreachable("not resolved"); } } + StringRef objcString; if (auto objcExpr = dyn_cast_or_null (E->getObjCStringLiteralExpr())) From 800feda1d77894a18c3d25a56c2ac65e25c41006 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 13:35:44 -0800 Subject: [PATCH 22/29] Fix spacing --- lib/SILGen/SILGenApply.cpp | 1 + lib/SILGen/SILGenExpr.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index fcb1c5e8721d7..2a004c70d69be 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6078,6 +6078,7 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( AbstractionPattern origFnType(substFnType); auto fnType = getLoweredType(origFnType, substFnType).castTo(); + SmallVector argValues; SmallVector delayedArgs; ArgEmitter emitter( diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index ada5eba9ab35e..7b4038d406e26 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3638,6 +3638,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { llvm_unreachable("not resolved"); } } + StringRef objcString; if (auto objcExpr = dyn_cast_or_null (E->getObjCStringLiteralExpr())) From 775c0625c68bc93ff93e08a54f2ab953c229f23b Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 13:36:52 -0800 Subject: [PATCH 23/29] Fix spacing (again) --- lib/SILGen/SILGenExpr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 7b4038d406e26..8b94e44bd4469 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -3638,7 +3638,7 @@ RValue RValueEmitter::visitKeyPathExpr(KeyPathExpr *E, SGFContext C) { llvm_unreachable("not resolved"); } } - + StringRef objcString; if (auto objcExpr = dyn_cast_or_null (E->getObjCStringLiteralExpr())) From 3a9845229560565ad5cd94dae18ddc23be24f305 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 12 Nov 2019 18:12:43 -0800 Subject: [PATCH 24/29] Add FileCheck tests --- lib/SILGen/SILGenApply.cpp | 4 ++-- lib/SILGen/SILGenFunction.h | 8 ++++++++ test/SILGen/keypaths.swift | 37 +++++++++++++++++++++++++++++++++++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 2a004c70d69be..87961ce4f6afe 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6082,10 +6082,10 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( SmallVector argValues; SmallVector delayedArgs; ArgEmitter emitter( - *this, fnType.getPointer()->getRepresentation(), + *this, fnType->getRepresentation(), /*yield*/ false, /*isForCoroutine*/ false, - ClaimedParamsRef(fnType, fnType.getPointer()->getParameters()), argValues, + ClaimedParamsRef(fnType, fnType->getParameters()), argValues, delayedArgs, /*foreign error*/ None, ImportAsMemberStatus()); diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index a0fdcc3bb40d3..91d616286dbbb 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -665,6 +665,14 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction 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 ParanExpr). SmallVector emitKeyPathSubscriptOperands(SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr); diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index 2c4cf966dce2d..f84bb016981f8 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -355,7 +355,21 @@ struct SubscriptDefaults3 { } } -// CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts +struct SubscriptDefaults4 { + subscript(x x: T, y y: T = 0) -> T { + get { fatalError() } + set { fatalError() } + } +} + +struct SubscriptDefaults5 { + subscript(x x: T, y y: T = #function) -> T { + get { fatalError() } + set { fatalError() } + } +} + +// CHECK-LABEL: sil hidden [ossa] @{{.*}}10subscripts1x1y1syx_q_SStSHRzSHR_r0_lF func subscripts(x: T, y: U, s: String) { _ = \Subscripts.[] _ = \Subscripts.[generic: x] @@ -398,9 +412,28 @@ func subscripts(x: T, y: U, s: String) { _ = \SubscriptDefaults2.[] _ = \SubscriptDefaults2.[0] - _ = \SubscriptDefaults3.[] _ = \SubscriptDefaults3.[0] + _ = \SubscriptDefaults5.[x: ""] + _ = \SubscriptDefaults5.[x: "", y: ""] +} + +// CHECK-LABEL: sil hidden [ossa] @{{.*}}check_default_subscripts +func check_default_subscripts() { + // CHECK: [[INTINIT:%[0-9]+]] = integer_literal $Builtin.Int64, 0 + // CHECK: [[I:%[0-9]+]] = struct $Int ([[INTINIT]] : $Builtin.Int64) + // CHECK: [[DFN:%[0-9]+]] = function_ref @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluipfA0_ : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric> () -> @out τ_0_0 + // CHECK: [[ALLOC:%[0-9]+]] = alloc_stack $Int + // CHECK: apply [[DFN]]([[ALLOC]]) : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric> () -> @out τ_0_0 + // CHECK: [[LOAD:%[0-9]+]] = load [[ALLOC]] : $*Int + // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath, (root $SubscriptDefaults4; settable_property $Int, id @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluig : $@convention(method) <τ_0_0 where τ_0_0 : Numeric> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults4) -> @out τ_0_0, getter @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTK : $@convention(thin) (@in_guaranteed SubscriptDefaults4, UnsafeRawPointer) -> @out Int, setter @$s3run18SubscriptDefaults4V1x1yxx_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: [[INTX:%[0-9]+]] = integer_literal $Builtin.Int64, 0 + // CHECK: [[IX:%[0-9]+]] = struct $Int ([[INTX]] : $Builtin.Int64) + // CHECK: [[INTY:%[0-9]+]] = integer_literal $Builtin.Int64, 0 + // CHECK: [[IY:%[0-9]+]] = struct $Int ([[INTY]] : $Builtin.Int64) + // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath, (root $SubscriptDefaults4; settable_property $Int, id @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluig : $@convention(method) <τ_0_0 where τ_0_0 : Numeric> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults4) -> @out τ_0_0, getter @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTK : $@convention(thin) (@in_guaranteed SubscriptDefaults4, UnsafeRawPointer) -> @out Int, setter @$s3run18SubscriptDefaults4V1x1yxx_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]], [[XY]]) + _ = \SubscriptDefaults4.[x: 0, y: 0] } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From 976c8c93100cc32c19a9b700f401d4ad30ae7c60 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 17 Nov 2019 13:44:15 -0800 Subject: [PATCH 25/29] Add tests --- test/SILGen/keypaths.swift | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index f84bb016981f8..bd932dcec5b29 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -404,7 +404,6 @@ func subscripts(x: T, y: U, s: String) { _ = \SubscriptDefaults1.[0] _ = \SubscriptDefaults1.[0, 0] _ = \SubscriptDefaults1.[0, 0, 0] - _ = \SubscriptDefaults1.[false] _ = \SubscriptDefaults1.[false, bool: false] _ = \SubscriptDefaults1.[bool: false, 0] @@ -414,26 +413,31 @@ func subscripts(x: T, y: U, s: String) { _ = \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: [[INTINIT:%[0-9]+]] = integer_literal $Builtin.Int64, 0 - // CHECK: [[I:%[0-9]+]] = struct $Int ([[INTINIT]] : $Builtin.Int64) - // CHECK: [[DFN:%[0-9]+]] = function_ref @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluipfA0_ : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric> () -> @out τ_0_0 + // 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, (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]]([[ALLOC]]) : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric> () -> @out τ_0_0 - // CHECK: [[LOAD:%[0-9]+]] = load [[ALLOC]] : $*Int - // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath, (root $SubscriptDefaults4; settable_property $Int, id @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluig : $@convention(method) <τ_0_0 where τ_0_0 : Numeric> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults4) -> @out τ_0_0, getter @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTK : $@convention(thin) (@in_guaranteed SubscriptDefaults4, UnsafeRawPointer) -> @out Int, setter @$s3run18SubscriptDefaults4V1x1yxx_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]]) + // CHECK: [[LOAD:%[0-9]+]] = load [trivial] [[ALLOC]] : $*Int + // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath, (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: [[INTX:%[0-9]+]] = integer_literal $Builtin.Int64, 0 - // CHECK: [[IX:%[0-9]+]] = struct $Int ([[INTX]] : $Builtin.Int64) - // CHECK: [[INTY:%[0-9]+]] = integer_literal $Builtin.Int64, 0 - // CHECK: [[IY:%[0-9]+]] = struct $Int ([[INTY]] : $Builtin.Int64) - // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath, (root $SubscriptDefaults4; settable_property $Int, id @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluig : $@convention(method) <τ_0_0 where τ_0_0 : Numeric> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_0, SubscriptDefaults4) -> @out τ_0_0, getter @$s3run18SubscriptDefaults4V1x1yxx_xtcSjRzluipACSiTK : $@convention(thin) (@in_guaranteed SubscriptDefaults4, UnsafeRawPointer) -> @out Int, setter @$s3run18SubscriptDefaults4V1x1yxx_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]], [[XY]]) - _ = \SubscriptDefaults4.[x: 0, y: 0] + + } // CHECK-LABEL: sil hidden [ossa] @{{.*}}subclass_generics From fd02663b790713994449630fa86e1e10b5432e8c Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 17 Nov 2019 14:29:05 -0800 Subject: [PATCH 26/29] More tests --- test/SILGen/Inputs/default_arg_other.swift | 12 +++++ .../SILGen/default_arg_multiple_modules.swift | 53 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 test/SILGen/Inputs/default_arg_other.swift create mode 100644 test/SILGen/default_arg_multiple_modules.swift diff --git a/test/SILGen/Inputs/default_arg_other.swift b/test/SILGen/Inputs/default_arg_other.swift new file mode 100644 index 0000000000000..d45733916862c --- /dev/null +++ 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 new file mode 100644 index 0000000000000..2deff092a9457 --- /dev/null +++ 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, (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, (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.[] +} From a8953b1f6d66ab120b27601d6c542e77ff8c7569 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Sun, 17 Nov 2019 14:29:26 -0800 Subject: [PATCH 27/29] Format with clang-format --- lib/SILGen/SILGenApply.cpp | 15 +++++++-------- lib/SILGen/SILGenFunction.h | 10 +++++----- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 87961ce4f6afe..af8a0f59e8d3b 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -6078,16 +6078,15 @@ SmallVector SILGenFunction::emitKeyPathSubscriptOperands( AbstractionPattern origFnType(substFnType); auto fnType = getLoweredType(origFnType, substFnType).castTo(); - + SmallVector argValues; SmallVector delayedArgs; - ArgEmitter emitter( - *this, fnType->getRepresentation(), - /*yield*/ false, - /*isForCoroutine*/ false, - ClaimedParamsRef(fnType, fnType->getParameters()), argValues, - delayedArgs, - /*foreign error*/ None, ImportAsMemberStatus()); + ArgEmitter emitter(*this, fnType->getRepresentation(), + /*yield*/ false, + /*isForCoroutine*/ false, + ClaimedParamsRef(fnType, fnType->getParameters()), + argValues, delayedArgs, + /*foreign error*/ None, ImportAsMemberStatus()); auto prepared = prepareSubscriptIndices(subscript, subs, diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 91d616286dbbb..9f647cc51d5fb 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -665,14 +665,14 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction IsFreeFunctionWitness_t isFree, bool isSelfConformance); - /// Generates subscript arguments for keypath. This function handles lowering of all index expressions - /// including default arguments. + /// 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 ParanExpr). + /// \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 ParanExpr). SmallVector emitKeyPathSubscriptOperands(SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr); From b8947c4e16196d9d3081f31e5cd340db37e2e0bf Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 18 Nov 2019 09:19:18 -0800 Subject: [PATCH 28/29] Add more FileCheck tests --- test/SILGen/keypaths.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/SILGen/keypaths.swift b/test/SILGen/keypaths.swift index bd932dcec5b29..38ea8c64e6c39 100644 --- a/test/SILGen/keypaths.swift +++ b/test/SILGen/keypaths.swift @@ -437,7 +437,19 @@ func check_default_subscripts() { // CHECK: [[KEYPATH:%[0-9]+]] = keypath $WritableKeyPath, (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, (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, (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 From 3f7ee854b8f0ddbbdd688b4f07d2fed62bffdb9f Mon Sep 17 00:00:00 2001 From: zoecarver Date: Mon, 18 Nov 2019 09:20:36 -0800 Subject: [PATCH 29/29] Fix nits --- lib/SILGen/SILGenFunction.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h index 9f647cc51d5fb..46c6676400de1 100644 --- a/lib/SILGen/SILGenFunction.h +++ b/lib/SILGen/SILGenFunction.h @@ -670,9 +670,9 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction /// /// \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 ParanExpr). + /// \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 emitKeyPathSubscriptOperands(SubscriptDecl *subscript, SubstitutionMap subs, Expr *indexExpr);