Skip to content

Commit 507d91c

Browse files
artemcmartemcm
authored
Merge pull request #61249 from apple/revert-61213-key-path-literal-as-function-ast-optimization
Revert "Sema: Simplify AST representation of key path literals as functions."
2 parents f7b1f71 + b8c1b4b commit 507d91c

File tree

6 files changed

+67
-111
lines changed

6 files changed

+67
-111
lines changed

‎include/swift/AST/Expr.h

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4047,6 +4047,14 @@ class ClosureExpr : public AbstractClosureExpr {
40474047
/// func f(x : @autoclosure () -> Int)
40484048
/// f(42) // AutoclosureExpr convert from Int to ()->Int
40494049
/// \endcode
4050+
///
4051+
/// They are also created when key path expressions are converted to function
4052+
/// type, in which case, a pair of nested implicit closures are formed:
4053+
/// \code
4054+
/// { $kp$ in { $0[keyPath: $kp$] } }( \(E) )
4055+
/// \endcode
4056+
/// This is to ensure side effects of the key path expression (mainly indices in
4057+
/// subscripts) are only evaluated once.
40504058
class AutoClosureExpr : public AbstractClosureExpr {
40514059
BraceStmt *Body;
40524060

@@ -5389,10 +5397,6 @@ class KeyPathExpr : public Expr {
53895397
/// a contextual root type.
53905398
bool HasLeadingDot = false;
53915399

5392-
/// When we parse a key path literal, we claim a closure discriminator for it, since it may be used as
5393-
/// a closure value in function type context.
5394-
unsigned ClosureDiscriminator;
5395-
53965400
public:
53975401
/// A single stored component, which will be one of:
53985402
/// - an unresolved DeclNameRef, which has to be type-checked
@@ -5724,12 +5728,11 @@ class KeyPathExpr : public Expr {
57245728

57255729
KeyPathExpr(SourceLoc startLoc, Expr *parsedRoot, Expr *parsedPath,
57265730
SourceLoc endLoc, bool hasLeadingDot, bool isObjC,
5727-
bool isImplicit, unsigned closureDiscriminator);
5731+
bool isImplicit);
57285732

57295733
/// Create a key path with unresolved root and path expressions.
57305734
KeyPathExpr(SourceLoc backslashLoc, Expr *parsedRoot, Expr *parsedPath,
5731-
bool hasLeadingDot, bool isImplicit,
5732-
unsigned closureDiscriminator);
5735+
bool hasLeadingDot, bool isImplicit);
57335736

57345737
/// Create a key path with components.
57355738
KeyPathExpr(ASTContext &ctx, SourceLoc startLoc,
@@ -5739,9 +5742,8 @@ class KeyPathExpr : public Expr {
57395742
public:
57405743
/// Create a new parsed Swift key path expression.
57415744
static KeyPathExpr *createParsed(ASTContext &ctx, SourceLoc backslashLoc,
5742-
Expr *parsedRoot, Expr *parsedPath,
5743-
bool hasLeadingDot,
5744-
unsigned closureDiscriminator = AbstractClosureExpr::InvalidDiscriminator);
5745+
Expr *parsedRoot, Expr *parsedPath,
5746+
bool hasLeadingDot);
57455747

57465748
/// Create a new parsed #keyPath expression.
57475749
static KeyPathExpr *createParsedPoundKeyPath(ASTContext &ctx,
@@ -5836,9 +5838,6 @@ class KeyPathExpr : public Expr {
58365838
/// True if this key path expression has a leading dot.
58375839
bool expectsContextualRoot() const { return HasLeadingDot; }
58385840

5839-
/// Return the discriminator to use if this key path becomes a closure.
5840-
unsigned getClosureDiscriminator() const { return ClosureDiscriminator; }
5841-
58425841
static bool classof(const Expr *E) {
58435842
return E->getKind() == ExprKind::KeyPath;
58445843
}

‎lib/AST/Expr.cpp

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2252,24 +2252,21 @@ OpenedArchetypeType *OpenExistentialExpr::getOpenedArchetype() const {
22522252

22532253
KeyPathExpr::KeyPathExpr(SourceLoc startLoc, Expr *parsedRoot,
22542254
Expr *parsedPath, SourceLoc endLoc, bool hasLeadingDot,
2255-
bool isObjC, bool isImplicit,
2256-
unsigned closureDiscriminator)
2255+
bool isObjC, bool isImplicit)
22572256
: Expr(ExprKind::KeyPath, isImplicit), StartLoc(startLoc), EndLoc(endLoc),
22582257
ParsedRoot(parsedRoot), ParsedPath(parsedPath),
2259-
HasLeadingDot(hasLeadingDot), ClosureDiscriminator(closureDiscriminator) {
2258+
HasLeadingDot(hasLeadingDot) {
22602259
assert(!(isObjC && (parsedRoot || parsedPath)) &&
22612260
"Obj-C key paths should only have components");
22622261
Bits.KeyPathExpr.IsObjC = isObjC;
22632262
}
22642263

22652264
KeyPathExpr::KeyPathExpr(SourceLoc backslashLoc, Expr *parsedRoot,
2266-
Expr *parsedPath, bool hasLeadingDot, bool isImplicit,
2267-
unsigned closureDiscriminator)
2265+
Expr *parsedPath, bool hasLeadingDot, bool isImplicit)
22682266
: KeyPathExpr(backslashLoc, parsedRoot, parsedPath,
22692267
parsedPath ? parsedPath->getEndLoc()
22702268
: parsedRoot->getEndLoc(),
2271-
hasLeadingDot, /*isObjC*/ false, isImplicit,
2272-
closureDiscriminator) {
2269+
hasLeadingDot, /*isObjC*/ false, isImplicit) {
22732270
assert((parsedRoot || parsedPath) &&
22742271
"Key path must have either root or path");
22752272
}
@@ -2278,8 +2275,7 @@ KeyPathExpr::KeyPathExpr(ASTContext &ctx, SourceLoc startLoc,
22782275
ArrayRef<Component> components, SourceLoc endLoc,
22792276
bool isObjC, bool isImplicit)
22802277
: KeyPathExpr(startLoc, /*parsedRoot*/ nullptr, /*parsedPath*/ nullptr,
2281-
endLoc, /*hasLeadingDot*/ false, isObjC, isImplicit,
2282-
/*closure discriminator*/ AbstractClosureExpr::InvalidDiscriminator) {
2278+
endLoc, /*hasLeadingDot*/ false, isObjC, isImplicit) {
22832279
assert(!components.empty());
22842280
Components = ctx.AllocateCopy(components);
22852281
}
@@ -2293,11 +2289,9 @@ KeyPathExpr *KeyPathExpr::createParsedPoundKeyPath(
22932289

22942290
KeyPathExpr *KeyPathExpr::createParsed(ASTContext &ctx, SourceLoc backslashLoc,
22952291
Expr *parsedRoot, Expr *parsedPath,
2296-
bool hasLeadingDot,
2297-
unsigned closureDiscriminator) {
2292+
bool hasLeadingDot) {
22982293
return new (ctx) KeyPathExpr(backslashLoc, parsedRoot, parsedPath,
2299-
hasLeadingDot, /*isImplicit*/ false,
2300-
closureDiscriminator);
2294+
hasLeadingDot, /*isImplicit*/ false);
23012295
}
23022296

23032297
KeyPathExpr *KeyPathExpr::createImplicit(ASTContext &ctx,
@@ -2313,8 +2307,7 @@ KeyPathExpr *KeyPathExpr::createImplicit(ASTContext &ctx,
23132307
Expr *parsedRoot, Expr *parsedPath,
23142308
bool hasLeadingDot) {
23152309
return new (ctx) KeyPathExpr(backslashLoc, parsedRoot, parsedPath,
2316-
hasLeadingDot, /*isImplicit*/ true,
2317-
/*closureDiscriminator*/ AbstractClosureExpr::InvalidDiscriminator);
2310+
hasLeadingDot, /*isImplicit*/ true);
23182311
}
23192312

23202313
void

‎lib/Parse/ParseExpr.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,8 +690,7 @@ ParserResult<Expr> Parser::parseExprKeyPath() {
690690

691691
auto *keypath = KeyPathExpr::createParsed(
692692
Context, backslashLoc, rootResult.getPtrOrNull(),
693-
pathResult.getPtrOrNull(), hasLeadingDot,
694-
CurLocalContext->claimNextClosureDiscriminator());
693+
pathResult.getPtrOrNull(), hasLeadingDot);
695694
return makeParserResult(parseStatus, keypath);
696695
}
697696

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,11 +1748,6 @@ static bool canPeepholeLiteralClosureConversion(Type literalType,
17481748

17491749
if (!literalFnType || !convertedFnType)
17501750
return false;
1751-
1752-
// Is it an identity conversion?
1753-
if (literalFnType->isEqual(convertedFnType)) {
1754-
return true;
1755-
}
17561751

17571752
// Does the conversion only add `throws`?
17581753
if (literalFnType->isEqual(convertedFnType->getWithoutThrowing())) {

‎lib/SILOptimizer/IPO/CapturePropagation.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -329,22 +329,6 @@ void CapturePropagation::rewritePartialApply(PartialApplyInst *OrigPAI,
329329
auto *T2TF = Builder.createThinToThickFunction(OrigPAI->getLoc(), FuncRef,
330330
OrigPAI->getType());
331331
OrigPAI->replaceAllUsesWith(T2TF);
332-
333-
// Bypass any mark_dependence on the captures we specialized away.
334-
//
335-
// TODO: If we start to specialize away key path literals with operands
336-
// (subscripts etc.), then a dependence of the new partial_apply on those
337-
// operands may still exist. However, we should still leave the key path
338-
// itself out of the dependency chain, and introduce dependencies on those
339-
// operands instead, so that the key path object itself can be made dead.
340-
for (auto user : T2TF->getUsersOfType<MarkDependenceInst>()) {
341-
if (auto depUser = user->getBase()->getSingleUserOfType<PartialApplyInst>()){
342-
if (depUser == OrigPAI) {
343-
user->replaceAllUsesWith(T2TF);
344-
}
345-
}
346-
}
347-
348332
// Remove any dealloc_stack users.
349333
SmallVector<Operand*, 16> Uses(T2TF->getUses());
350334
for (auto *Use : Uses)

‎lib/Sema/CSApply.cpp

Lines changed: 46 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4971,72 +4971,53 @@ namespace {
49714971
cs.cacheType(E);
49724972

49734973
// To ensure side effects of the key path expression (mainly indices in
4974-
// subscripts) are only evaluated once, we use a capture list to evaluate
4975-
// the key path immediately and capture it in the function value created.
4976-
// The result looks like:
4974+
// subscripts) are only evaluated once, we construct an outer closure,
4975+
// which is immediately evaluated, and an inner closure, which it returns.
4976+
// The result looks like this:
49774977
//
4978-
// return "{ [$kp$ = \(E)] in $0[keyPath: $kp$] }"
4978+
// return "{ $kp$ in { $0[keyPath: $kp$] } }( \(E) )"
49794979

49804980
auto &ctx = cs.getASTContext();
4981-
auto discriminator = E->getClosureDiscriminator();
4981+
auto discriminator = AutoClosureExpr::InvalidDiscriminator;
49824982

4983-
auto contextInfo = exprType->castTo<AnyFunctionType>()->getExtInfo();
4983+
// The inner closure.
4984+
//
4985+
// let closure = "{ $0[keyPath: $kp$] }"
4986+
//
4987+
// FIXME: Verify ExtInfo state is correct, not working by accident.
49844988
FunctionType::ExtInfo closureInfo;
4985-
closureInfo = closureInfo.withNoEscape(contextInfo.isNoEscape());
49864989
auto closureTy =
49874990
FunctionType::get({FunctionType::Param(baseTy)}, leafTy, closureInfo);
4988-
4989-
ClosureExpr *closure = new (ctx)
4990-
ClosureExpr(DeclAttributes(),
4991-
SourceRange(),
4992-
/*captured self*/ nullptr,
4993-
/*params*/ nullptr,
4994-
SourceLoc(),
4995-
SourceLoc(),
4996-
SourceLoc(),
4997-
SourceLoc(),
4998-
/*explicit result type*/ nullptr,
4999-
discriminator,
5000-
dc);
5001-
4991+
auto closure = new (ctx)
4992+
AutoClosureExpr(/*set body later*/nullptr, leafTy,
4993+
discriminator, dc);
50024994
auto param = new (ctx) ParamDecl(
50034995
SourceLoc(),
50044996
/*argument label*/ SourceLoc(), Identifier(),
50054997
/*parameter name*/ SourceLoc(), ctx.getIdentifier("$0"), closure);
50064998
param->setInterfaceType(baseTy->mapTypeOutOfContext());
50074999
param->setSpecifier(ParamSpecifier::Default);
50085000
param->setImplicit();
5009-
5010-
auto params = ParameterList::create(ctx, SourceLoc(),
5011-
param, SourceLoc());
50125001

5013-
closure->setParameterList(params);
5014-
5015-
// The capture list.
5016-
VarDecl *outerParam = new (ctx) VarDecl(/*static*/ false,
5017-
VarDecl::Introducer::Let,
5018-
SourceLoc(),
5019-
ctx.getIdentifier("$kp$"),
5020-
dc);
5021-
outerParam->setImplicit();
5002+
// The outer closure.
5003+
//
5004+
// let outerClosure = "{ $kp$ in \(closure) }"
5005+
//
5006+
// FIXME: Verify ExtInfo state is correct, not working by accident.
5007+
FunctionType::ExtInfo outerClosureInfo;
5008+
auto outerClosureTy = FunctionType::get({FunctionType::Param(keyPathTy)},
5009+
closureTy, outerClosureInfo);
5010+
auto outerClosure = new (ctx)
5011+
AutoClosureExpr(/*set body later*/nullptr, closureTy,
5012+
discriminator, dc);
5013+
auto outerParam =
5014+
new (ctx) ParamDecl(SourceLoc(),
5015+
/*argument label*/ SourceLoc(), Identifier(),
5016+
/*parameter name*/ SourceLoc(),
5017+
ctx.getIdentifier("$kp$"), outerClosure);
50225018
outerParam->setInterfaceType(keyPathTy->mapTypeOutOfContext());
5023-
5024-
NamedPattern *outerParamPat = new (ctx) NamedPattern(outerParam);
5025-
outerParamPat->setImplicit();
5026-
outerParamPat->setType(keyPathTy);
5027-
5028-
auto outerParamPBE = PatternBindingEntry(outerParamPat,
5029-
SourceLoc(), E, dc);
5030-
solution.setExprTypes(E);
5031-
auto outerParamDecl = PatternBindingDecl::create(ctx, SourceLoc(),
5032-
{}, SourceLoc(),
5033-
outerParamPBE,
5034-
dc);
5035-
outerParamDecl->setImplicit();
5036-
auto outerParamCapture = CaptureListEntry(outerParamDecl);
5037-
auto captureExpr = CaptureListExpr::create(ctx, outerParamCapture,
5038-
closure);
5039-
captureExpr->setImplicit();
5019+
outerParam->setSpecifier(ParamSpecifier::Default);
5020+
outerParam->setImplicit();
50405021

50415022
// let paramRef = "$0"
50425023
auto *paramRef = new (ctx)
@@ -5056,21 +5037,26 @@ namespace {
50565037
E->getStartLoc(), outerParamRef, E->getEndLoc(),
50575038
leafTy, /*implicit=*/true);
50585039
cs.cacheType(application);
5059-
5060-
auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), application);
5061-
auto bodyStmt = BraceStmt::create(ctx,
5062-
SourceLoc(), ASTNode(returnStmt), SourceLoc());
50635040

50645041
// Finish up the inner closure.
50655042
closure->setParameterList(ParameterList::create(ctx, {param}));
5066-
closure->setBody(bodyStmt, /*singleExpr*/ true);
5043+
closure->setBody(application);
50675044
closure->setType(closureTy);
50685045
cs.cacheType(closure);
5069-
5070-
captureExpr->setType(closureTy);
5071-
cs.cacheType(captureExpr);
5072-
5073-
return coerceToType(captureExpr, exprType, cs.getConstraintLocator(E));
5046+
5047+
// Finish up the outer closure.
5048+
outerClosure->setParameterList(ParameterList::create(ctx, {outerParam}));
5049+
outerClosure->setBody(closure);
5050+
outerClosure->setType(outerClosureTy);
5051+
cs.cacheType(outerClosure);
5052+
5053+
// let outerApply = "\( outerClosure )( \(E) )"
5054+
auto *argList = ArgumentList::forImplicitUnlabeled(ctx, {E});
5055+
auto outerApply = CallExpr::createImplicit(ctx, outerClosure, argList);
5056+
outerApply->setType(closureTy);
5057+
cs.cacheExprTypes(outerApply);
5058+
5059+
return coerceToType(outerApply, exprType, cs.getConstraintLocator(E));
50745060
}
50755061

50765062
void buildKeyPathOptionalForceComponent(

0 commit comments

Comments
 (0)