Skip to content

Commit 9e39efb

Browse files
nate-chandlernate-chandler
committed
WIP: Allow return to be omitted from single expression functions.
1 parent beca73b commit 9e39efb

File tree

7 files changed

+1438
-6
lines changed

7 files changed

+1438
-6
lines changed

‎include/swift/AST/Decl.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,8 @@ class alignas(1 << DeclAlignInBits) Decl {
400400
/// The ResilienceExpansion to use for default arguments.
401401
DefaultArgumentResilienceExpansion : 1
402402
);
403-
404-
SWIFT_INLINE_BITFIELD(AbstractFunctionDecl, ValueDecl, 3+8+1+1+1+1+1+1+1,
403+
404+
SWIFT_INLINE_BITFIELD(AbstractFunctionDecl, ValueDecl, 3+8+1+1+1+1+1+1+1+1,
405405
/// \see AbstractFunctionDecl::BodyKind
406406
BodyKind : 3,
407407

@@ -428,7 +428,10 @@ class alignas(1 << DeclAlignInBits) Decl {
428428

429429
/// Whether this member was synthesized as part of a derived
430430
/// protocol conformance.
431-
Synthesized : 1
431+
Synthesized : 1,
432+
433+
/// Whether this member's body consists of a single expression.
434+
HasSingleExpressionBody : 1
432435
);
433436

434437
SWIFT_INLINE_BITFIELD(FuncDecl, AbstractFunctionDecl, 1+2+1+1+2,
@@ -5433,13 +5436,25 @@ class AbstractFunctionDecl : public GenericContext, public ValueDecl {
54335436
Bits.AbstractFunctionDecl.DefaultArgumentResilienceExpansion =
54345437
unsigned(ResilienceExpansion::Maximal);
54355438
Bits.AbstractFunctionDecl.Synthesized = false;
5439+
Bits.AbstractFunctionDecl.HasSingleExpressionBody = false;
54365440
}
54375441

54385442
void setBodyKind(BodyKind K) {
54395443
Bits.AbstractFunctionDecl.BodyKind = unsigned(K);
54405444
}
54415445

54425446
public:
5447+
void setHasSingleExpressionBody(bool Has = true) {
5448+
Bits.AbstractFunctionDecl.HasSingleExpressionBody = Has;
5449+
}
5450+
5451+
bool hasSingleExpressionBody() const {
5452+
return Bits.AbstractFunctionDecl.HasSingleExpressionBody;
5453+
}
5454+
5455+
Expr *getSingleExpressionBody() const;
5456+
void setSingleExpressionBody(Expr *NewBody);
5457+
54435458
/// Returns the string for the base name, or "_" if this is unnamed.
54445459
StringRef getNameStr() const {
54455460
assert(!getFullName().isSpecial() && "Cannot get string for special names");

‎lib/AST/ASTDumper.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1069,7 +1069,10 @@ namespace {
10691069
Indent -= 2;
10701070
}
10711071
}
1072-
if (auto Body = D->getBody(/*canSynthesize=*/false)) {
1072+
if (D->hasSingleExpressionBody()) {
1073+
OS << '\n';
1074+
printRec(D->getSingleExpressionBody());
1075+
} else if (auto Body = D->getBody(/*canSynthesize=*/false)) {
10731076
OS << '\n';
10741077
printRec(Body, D->getASTContext());
10751078
}

‎lib/AST/ASTWalker.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,15 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
339339
}
340340
}
341341

342+
// FIXME: FIXME_NOW: DETERMINE: Is this or something like it needed? Right
343+
// now it's causing assertion failures during AST verification.
344+
//
345+
// if (AFD->hasSingleExpressionBody()) {
346+
// if (Expr *body = doIt(AFD->getSingleExpressionBody()))
347+
// AFD->setSingleExpressionBody(body);
348+
// else
349+
// return true;
350+
// }
342351
if (AFD->getBody(/*canSynthesize=*/false)) {
343352
AbstractFunctionDecl::BodyKind PreservedKind = AFD->getBodyKind();
344353
if (BraceStmt *S = cast_or_null<BraceStmt>(doIt(AFD->getBody())))

‎lib/AST/Decl.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,26 @@ case DeclKind::ID: return cast<ID##Decl>(this)->getLoc();
461461
llvm_unreachable("Unknown decl kind");
462462
}
463463

464+
Expr *AbstractFunctionDecl::getSingleExpressionBody() const {
465+
assert(hasSingleExpressionBody() && "Not a single-expression body");
466+
auto braceStmt = getBody();
467+
assert(braceStmt != nullptr && "No body currently available.");
468+
auto body = getBody()->getElement(0);
469+
if (body.is<Stmt *>())
470+
return cast<ReturnStmt>(body.get<Stmt *>())->getResult();
471+
return body.get<Expr *>();
472+
}
473+
474+
void AbstractFunctionDecl::setSingleExpressionBody(Expr *NewBody) {
475+
assert(hasSingleExpressionBody() && "Not a single-expression body");
476+
auto body = getBody()->getElement(0);
477+
if (body.is<Stmt *>()) {
478+
cast<ReturnStmt>(body.get<Stmt *>())->setResult(NewBody);
479+
return;
480+
}
481+
getBody()->setElement(0, NewBody);
482+
}
483+
464484
bool AbstractStorageDecl::isTransparent() const {
465485
return getAttrs().hasAttribute<TransparentAttr>();
466486
}

‎lib/Parse/ParseDecl.cpp

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5636,8 +5636,56 @@ void Parser::parseAbstractFunctionBody(AbstractFunctionDecl *AFD) {
56365636
Context.Stats->getFrontendCounters().NumFunctionsParsed++;
56375637

56385638
ParserResult<BraceStmt> Body = parseBraceItemList(diag::invalid_diagnostic);
5639-
if (!Body.isNull())
5640-
AFD->setBody(Body.get());
5639+
if (!Body.isNull()) {
5640+
// If the body consists of a single expression, turn it into a return
5641+
// statement.
5642+
BraceStmt * BS = Body.get();
5643+
AFD->setBody(BS);
5644+
// FIXME: FIXME_NOW: DETERMINE: Do we need to handle the code completion
5645+
// case here? It is being handled for closure
5646+
// (Parser::parseExprClosure).
5647+
if (BS->getNumElements() == 1) {
5648+
auto Element = BS->getElement(0);
5649+
// FIXME: FIXME_NOW: DETERMINE: Do we need to do this? It's being done
5650+
// for closures (Parser::parseExprClosure) but it may not be
5651+
// relevant here.
5652+
//
5653+
// if (Element.is<Stmt *>()) {
5654+
// if (auto returnStmt =
5655+
// dyn_cast_or_null<ReturnStmt>(Element.get<Stmt*>())) {
5656+
//
5657+
// if (!returnStmt->hasResult()) {
5658+
// Expr *returnExpr = TupleExpr::createEmpty(Context,
5659+
// SourceLoc(),
5660+
// SourceLoc(),
5661+
// /*implicit*/true);
5662+
//
5663+
// returnStmt->setResult(returnExpr);
5664+
// AFD->setHasSingleExpressionBody();
5665+
// AFD->setSingleExpressionBody(returnExpr);
5666+
// }
5667+
// }
5668+
// } else
5669+
if (Element.is<Expr *>()) {
5670+
auto E = Element.get<Expr *>();
5671+
if (auto F = dyn_cast_or_null<FuncDecl>(AFD)) {
5672+
auto RS = new (Context) ReturnStmt(SourceLoc(), E);
5673+
BS->setElement(0, RS);
5674+
AFD->setHasSingleExpressionBody();
5675+
AFD->setSingleExpressionBody(E);
5676+
} else if (auto F = dyn_cast_or_null<ConstructorDecl>(AFD)) {
5677+
if (F->getFailability() != OTK_None && isa<NilLiteralExpr>(E)) {
5678+
// If it's a nil literal, just insert return. This is the only
5679+
// legal thing to return.
5680+
auto RS = new (Context) ReturnStmt(E->getStartLoc(), E);
5681+
BS->setElement(0, RS);
5682+
AFD->setHasSingleExpressionBody();
5683+
AFD->setSingleExpressionBody(E);
5684+
}
5685+
}
5686+
}
5687+
}
5688+
}
56415689
}
56425690

56435691
bool Parser::parseAbstractFunctionBodyDelayed(AbstractFunctionDecl *AFD) {

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1920,6 +1920,48 @@ bool TypeChecker::typeCheckFunctionBodyUntil(FuncDecl *FD,
19201920
BraceStmt *BS = FD->getBody();
19211921
assert(BS && "Should have a body");
19221922

1923+
// FIXME: FIXME_NOW: DETERMINE: What is the appropriate place for this code to
1924+
// live?
1925+
if (FD->hasSingleExpressionBody()) {
1926+
auto fnType = FD->getBodyResultTypeLoc().getType();
1927+
auto returnStmt = cast<ReturnStmt>(BS->getElement(0).get<Stmt *>());
1928+
auto E = returnStmt->getResult();
1929+
1930+
// FIXME: FIXME_NOW: DETERMINE: Does this actually need to be here? It's
1931+
// being done for closures, but there may be a reason that applies
1932+
// there but not here.
1933+
//
1934+
// if (E != FD->getSingleExpressionBody()) {
1935+
// FD->setSingleExpressionBody(E);
1936+
// }
1937+
1938+
if (FD->getBodyResultTypeLoc().isNull() || fnType->isVoid()) {
1939+
auto voidExpr = TupleExpr::createEmpty(Context,
1940+
E->getStartLoc(),
1941+
E->getEndLoc(),
1942+
/*implicit*/true);
1943+
returnStmt->setResult(voidExpr);
1944+
BS = BraceStmt::create(Context,
1945+
BS->getStartLoc(),
1946+
{ E, returnStmt },
1947+
BS->getEndLoc(),
1948+
/*implicit=*/true);
1949+
} else {
1950+
// FIXME: FIXME_NOW: DETERMINE: Is this the appropriate mechanism to use
1951+
// to divine the type of E?
1952+
auto exprTy = typeCheckExpression(E, FD, TypeLoc(),
1953+
CTP_ReturnStmt);
1954+
if (!exprTy.isNull() && exprTy->isUninhabited() &&
1955+
exprTy->getCanonicalType() != fnType->getCanonicalType()) {
1956+
BS = BraceStmt::create(Context,
1957+
BS->getStartLoc(),
1958+
{ E },
1959+
BS->getEndLoc(),
1960+
/*implicit=*/true);
1961+
}
1962+
}
1963+
}
1964+
19231965
StmtChecker SC(*this, static_cast<AbstractFunctionDecl *>(FD));
19241966
SC.EndTypeCheckLoc = EndTypeCheckLoc;
19251967
bool HadError = SC.typeCheckBody(BS);

0 commit comments

Comments
 (0)