Skip to content

Commit 51bd8d9

Browse files
xedinxedin
authored
Merge pull request #34510 from maustinstar/sr-11711
[SR-11711] [Parse] Single-expression implicit returns within #if declarations
2 parents a2d7c70 + d0f8c96 commit 51bd8d9

16 files changed

+2918
-77
lines changed

‎include/swift/AST/Stmt.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ class BraceStmt final : public Stmt,
180180
ASTNode getLastElement() const { return getElements().back(); }
181181

182182
void setFirstElement(ASTNode node) { getElements().front() = node; }
183+
void setLastElement(ASTNode node) { getElements().back() = node; }
183184

184185
/// The elements contained within the BraceStmt.
185186
MutableArrayRef<ASTNode> getElements() {

‎include/swift/Parse/Parser.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,11 @@ class Parser {
710710
/// \param DiagText name for the string literal in the diagnostic.
711711
Optional<StringRef>
712712
getStringLiteralIfNotInterpolated(SourceLoc Loc, StringRef DiagText);
713+
714+
/// Returns true when body elements are eligible as single-expression implicit returns.
715+
///
716+
/// \param Body elements to search for implicit single-expression returns.
717+
bool shouldReturnSingleExpressionElement(ArrayRef<ASTNode> Body);
713718

714719
/// Returns true to indicate that experimental concurrency syntax should be
715720
/// parsed if the parser is generating only a syntax tree or if the user has

‎lib/AST/Decl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ Expr *AbstractFunctionDecl::getSingleExpressionBody() const {
684684
assert(hasSingleExpressionBody() && "Not a single-expression body");
685685
auto braceStmt = getBody();
686686
assert(braceStmt != nullptr && "No body currently available.");
687-
auto body = getBody()->getFirstElement();
687+
auto body = getBody()->getLastElement();
688688
if (auto *stmt = body.dyn_cast<Stmt *>()) {
689689
if (auto *returnStmt = dyn_cast<ReturnStmt>(stmt)) {
690690
return returnStmt->getResult();
@@ -701,7 +701,7 @@ Expr *AbstractFunctionDecl::getSingleExpressionBody() const {
701701

702702
void AbstractFunctionDecl::setSingleExpressionBody(Expr *NewBody) {
703703
assert(hasSingleExpressionBody() && "Not a single-expression body");
704-
auto body = getBody()->getFirstElement();
704+
auto body = getBody()->getLastElement();
705705
if (auto *stmt = body.dyn_cast<Stmt *>()) {
706706
if (auto *returnStmt = dyn_cast<ReturnStmt>(stmt)) {
707707
returnStmt->setResult(NewBody);
@@ -717,7 +717,7 @@ void AbstractFunctionDecl::setSingleExpressionBody(Expr *NewBody) {
717717
return;
718718
}
719719
}
720-
getBody()->setFirstElement(NewBody);
720+
getBody()->setLastElement(NewBody);
721721
}
722722

723723
bool AbstractStorageDecl::isTransparent() const {

‎lib/AST/Expr.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1974,10 +1974,10 @@ FORWARD_SOURCE_LOCS_TO(ClosureExpr, Body.getPointer())
19741974

19751975
Expr *ClosureExpr::getSingleExpressionBody() const {
19761976
assert(hasSingleExpressionBody() && "Not a single-expression body");
1977-
auto body = getBody()->getFirstElement();
1977+
auto body = getBody()->getLastElement();
19781978
if (auto stmt = body.dyn_cast<Stmt *>()) {
19791979
if (auto braceStmt = dyn_cast<BraceStmt>(stmt))
1980-
return braceStmt->getFirstElement().get<Expr *>();
1980+
return braceStmt->getLastElement().get<Expr *>();
19811981

19821982
return cast<ReturnStmt>(stmt)->getResult();
19831983
}
@@ -2003,7 +2003,7 @@ void AutoClosureExpr::setBody(Expr *E) {
20032003
}
20042004

20052005
Expr *AutoClosureExpr::getSingleExpressionBody() const {
2006-
return cast<ReturnStmt>(Body->getFirstElement().get<Stmt *>())->getResult();
2006+
return cast<ReturnStmt>(Body->getLastElement().get<Stmt *>())->getResult();
20072007
}
20082008

20092009
Expr *AutoClosureExpr::getUnwrappedCurryThunkExpr() const {

‎lib/IDE/CodeCompletion.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6171,13 +6171,13 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) {
61716171
// to work via TypeCheckCompletionCallback.
61726172
static void undoSingleExpressionReturn(DeclContext *DC) {
61736173
auto updateBody = [](BraceStmt *BS, ASTContext &Ctx) -> bool {
6174-
ASTNode FirstElem = BS->getFirstElement();
6175-
auto *RS = dyn_cast_or_null<ReturnStmt>(FirstElem.dyn_cast<Stmt*>());
6174+
ASTNode LastElem = BS->getLastElement();
6175+
auto *RS = dyn_cast_or_null<ReturnStmt>(LastElem.dyn_cast<Stmt*>());
61766176

61776177
if (!RS || !RS->isImplicit())
61786178
return false;
61796179

6180-
BS->setFirstElement(RS->getResult());
6180+
BS->setLastElement(RS->getResult());
61816181
return true;
61826182
};
61836183

‎lib/IDE/ExprContextAnalysis.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1113,7 +1113,24 @@ class ExprContextAnalyzer {
11131113
/// in order to avoid a base expression affecting the type. However, now that
11141114
/// we've typechecked, we will take the context type into account.
11151115
static bool isSingleExpressionBodyForCodeCompletion(BraceStmt *body) {
1116-
return body->getNumElements() == 1 && body->getFirstElement().is<Expr *>();
1116+
if (body->getNumElements() == 2) {
1117+
if (auto *D = body->getFirstElement().dyn_cast<Decl *>()) {
1118+
// Step into nested active clause.
1119+
while (auto *ICD = dyn_cast<IfConfigDecl>(D)) {
1120+
auto ACE = ICD->getActiveClauseElements();
1121+
if (ACE.size() == 1) {
1122+
return body->getLastElement().is<Expr *>();
1123+
} else if (ACE.size() == 2) {
1124+
if (auto *ND = ACE.front().dyn_cast<Decl *>()) {
1125+
D = ND;
1126+
continue;
1127+
}
1128+
}
1129+
break;
1130+
}
1131+
}
1132+
}
1133+
return body->getNumElements() == 1 && body->getLastElement().is<Expr *>();
11171134
}
11181135

11191136
public:

‎lib/Parse/ParseDecl.cpp

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6683,48 +6683,45 @@ BraceStmt *Parser::parseAbstractFunctionBodyImpl(AbstractFunctionDecl *AFD) {
66836683

66846684
BraceStmt *BS = Body.get();
66856685
AFD->setBodyParsed(BS);
6686-
6687-
// If the body consists of a single expression, turn it into a return
6688-
// statement.
6689-
if (BS->getNumElements() != 1)
6690-
return BS;
6691-
6692-
auto Element = BS->getFirstElement();
6693-
if (auto *stmt = Element.dyn_cast<Stmt *>()) {
6694-
if (isa<FuncDecl>(AFD)) {
6695-
if (auto *returnStmt = dyn_cast<ReturnStmt>(stmt)) {
6696-
if (!returnStmt->hasResult()) {
6697-
auto returnExpr = TupleExpr::createEmpty(Context,
6698-
SourceLoc(),
6699-
SourceLoc(),
6700-
/*implicit*/true);
6701-
returnStmt->setResult(returnExpr);
6702-
AFD->setHasSingleExpressionBody();
6703-
AFD->setSingleExpressionBody(returnExpr);
6686+
6687+
if (Parser::shouldReturnSingleExpressionElement(BS->getElements())) {
6688+
auto Element = BS->getLastElement();
6689+
if (auto *stmt = Element.dyn_cast<Stmt *>()) {
6690+
if (isa<FuncDecl>(AFD)) {
6691+
if (auto *returnStmt = dyn_cast<ReturnStmt>(stmt)) {
6692+
if (!returnStmt->hasResult()) {
6693+
auto returnExpr = TupleExpr::createEmpty(Context,
6694+
SourceLoc(),
6695+
SourceLoc(),
6696+
/*implicit*/true);
6697+
returnStmt->setResult(returnExpr);
6698+
AFD->setHasSingleExpressionBody();
6699+
AFD->setSingleExpressionBody(returnExpr);
6700+
}
67046701
}
67056702
}
6706-
}
6707-
} else if (auto *E = Element.dyn_cast<Expr *>()) {
6708-
if (auto SE = dyn_cast<SequenceExpr>(E->getSemanticsProvidingExpr())) {
6709-
if (SE->getNumElements() > 1 && isa<AssignExpr>(SE->getElement(1))) {
6710-
// This is an assignment. We don't want to implicitly return
6711-
// it.
6712-
return BS;
6703+
} else if (auto *E = Element.dyn_cast<Expr *>()) {
6704+
if (auto SE = dyn_cast<SequenceExpr>(E->getSemanticsProvidingExpr())) {
6705+
if (SE->getNumElements() > 1 && isa<AssignExpr>(SE->getElement(1))) {
6706+
// This is an assignment. We don't want to implicitly return
6707+
// it.
6708+
return BS;
6709+
}
67136710
}
6714-
}
6715-
if (isa<FuncDecl>(AFD)) {
6716-
auto RS = new (Context) ReturnStmt(SourceLoc(), E);
6717-
BS->setFirstElement(RS);
6718-
AFD->setHasSingleExpressionBody();
6719-
AFD->setSingleExpressionBody(E);
6720-
} else if (auto *F = dyn_cast<ConstructorDecl>(AFD)) {
6721-
if (F->isFailable() && isa<NilLiteralExpr>(E)) {
6722-
// If it's a nil literal, just insert return. This is the only
6723-
// legal thing to return.
6724-
auto RS = new (Context) ReturnStmt(E->getStartLoc(), E);
6725-
BS->setFirstElement(RS);
6711+
if (isa<FuncDecl>(AFD)) {
6712+
auto RS = new (Context) ReturnStmt(SourceLoc(), E);
6713+
BS->setLastElement(RS);
67266714
AFD->setHasSingleExpressionBody();
67276715
AFD->setSingleExpressionBody(E);
6716+
} else if (auto *F = dyn_cast<ConstructorDecl>(AFD)) {
6717+
if (F->isFailable() && isa<NilLiteralExpr>(E)) {
6718+
// If it's a nil literal, just insert return. This is the only
6719+
// legal thing to return.
6720+
auto RS = new (Context) ReturnStmt(E->getStartLoc(), E);
6721+
BS->setLastElement(RS);
6722+
AFD->setHasSingleExpressionBody();
6723+
AFD->setSingleExpressionBody(E);
6724+
}
67286725
}
67296726
}
67306727
}

‎lib/Parse/ParseExpr.cpp

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,39 +2839,30 @@ ParserResult<Expr> Parser::parseExprClosure() {
28392839
closure->setParameterList(params);
28402840
closure->setHasAnonymousClosureVars();
28412841
}
2842-
2842+
28432843
// If the body consists of a single expression, turn it into a return
28442844
// statement.
28452845
bool hasSingleExpressionBody = false;
2846-
if (!missingRBrace && bodyElements.size() == 1) {
2847-
// If the closure's only body element is a single return statement,
2848-
// use that instead of creating a new wrapping return expression.
2849-
Expr *returnExpr = nullptr;
2846+
if (!missingRBrace &&
2847+
Parser::shouldReturnSingleExpressionElement(bodyElements)) {
2848+
auto Element = bodyElements.back();
28502849

2851-
if (bodyElements[0].is<Stmt *>()) {
2852-
if (auto returnStmt =
2853-
dyn_cast<ReturnStmt>(bodyElements[0].get<Stmt*>())) {
2854-
2850+
if (Element.is<Stmt *>()) {
2851+
if (auto returnStmt = dyn_cast<ReturnStmt>(Element.get<Stmt *>())) {
2852+
hasSingleExpressionBody = true;
28552853
if (!returnStmt->hasResult()) {
2856-
2857-
returnExpr = TupleExpr::createEmpty(Context,
2858-
SourceLoc(),
2859-
SourceLoc(),
2860-
/*implicit*/true);
2861-
2854+
auto returnExpr = TupleExpr::createEmpty(Context,
2855+
SourceLoc(),
2856+
SourceLoc(),
2857+
/*implicit*/true);
28622858
returnStmt->setResult(returnExpr);
28632859
}
2864-
2865-
hasSingleExpressionBody = true;
28662860
}
2867-
}
2868-
2869-
// Otherwise, create the wrapping return.
2870-
if (bodyElements[0].is<Expr *>()) {
2861+
} else if (Element.is<Expr *>()) {
2862+
// Create the wrapping return.
28712863
hasSingleExpressionBody = true;
2872-
returnExpr = bodyElements[0].get<Expr*>();
2873-
bodyElements[0] = new (Context) ReturnStmt(SourceLoc(),
2874-
returnExpr);
2864+
auto returnExpr = Element.get<Expr*>();
2865+
bodyElements.back() = new (Context) ReturnStmt(SourceLoc(), returnExpr);
28752866
}
28762867
}
28772868

‎lib/Parse/Parser.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,6 +1185,31 @@ Parser::getStringLiteralIfNotInterpolated(SourceLoc Loc,
11851185
Segments.front().Length));
11861186
}
11871187

1188+
bool Parser::shouldReturnSingleExpressionElement(ArrayRef<ASTNode> Body) {
1189+
// If the body consists of an #if declaration with a single
1190+
// expression active clause, find a single expression.
1191+
if (Body.size() == 2) {
1192+
if (auto *D = Body.front().dyn_cast<Decl *>()) {
1193+
// Step into nested active clause.
1194+
while (auto *ICD = dyn_cast<IfConfigDecl>(D)) {
1195+
auto ACE = ICD->getActiveClauseElements();
1196+
if (ACE.size() == 1) {
1197+
assert(Body.back() == ACE.back() &&
1198+
"active clause not found in body");
1199+
return true;
1200+
} else if (ACE.size() == 2) {
1201+
if (auto *ND = ACE.front().dyn_cast<Decl *>()) {
1202+
D = ND;
1203+
continue;
1204+
}
1205+
}
1206+
break;
1207+
}
1208+
}
1209+
}
1210+
return Body.size() == 1;
1211+
}
1212+
11881213
struct ParserUnit::Implementation {
11891214
std::shared_ptr<SyntaxParseActions> SPActions;
11901215
LangOptions LangOpts;

‎lib/Sema/CSClosure.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,11 @@ class ClosureConstraintApplication
173173
}
174174

175175
void visitDecl(Decl *decl) {
176-
llvm_unreachable("Declarations not supported");
176+
177+
if (isa<IfConfigDecl>(decl))
178+
return;
179+
180+
llvm_unreachable("Unimplemented case for closure body");
177181
}
178182

179183
ASTNode visitBraceStmt(BraceStmt *braceStmt) {

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,7 +1939,7 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(Evaluator &evaluator,
19391939
func->getResultInterfaceType()->isVoid()) {
19401940
// The function returns void. We don't need an explicit return, no matter
19411941
// what the type of the expression is. Take the inserted return back out.
1942-
func->getBody()->setFirstElement(func->getSingleExpressionBody());
1942+
func->getBody()->setLastElement(func->getSingleExpressionBody());
19431943
}
19441944
}
19451945

@@ -2006,7 +2006,7 @@ TypeCheckFunctionBodyRequest::evaluate(Evaluator &evaluator,
20062006
func->getResultInterfaceType()->isVoid()) {
20072007
// The function returns void. We don't need an explicit return, no matter
20082008
// what the type of the expression is. Take the inserted return back out.
2009-
body->setFirstElement(func->getSingleExpressionBody());
2009+
body->setLastElement(func->getSingleExpressionBody());
20102010
}
20112011
} else if (isa<ConstructorDecl>(AFD) &&
20122012
(body->empty() ||
@@ -2040,12 +2040,12 @@ TypeCheckFunctionBodyRequest::evaluate(Evaluator &evaluator,
20402040
// that we have eagerly converted something like `{ fatalError() }`
20412041
// into `{ return fatalError() }` that has to be corrected here.
20422042
if (isa<FuncDecl>(AFD) && cast<FuncDecl>(AFD)->hasSingleExpressionBody()) {
2043-
if (auto *stmt = body->getFirstElement().dyn_cast<Stmt *>()) {
2043+
if (auto *stmt = body->getLastElement().dyn_cast<Stmt *>()) {
20442044
if (auto *retStmt = dyn_cast<ReturnStmt>(stmt)) {
20452045
if (retStmt->isImplicit() && retStmt->hasResult()) {
20462046
auto returnType = retStmt->getResult()->getType();
20472047
if (returnType && returnType->isUninhabited())
2048-
body->setFirstElement(retStmt->getResult());
2048+
body->setLastElement(retStmt->getResult());
20492049
}
20502050
}
20512051
}

‎test/FixCode/fixits-omit-return.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,16 @@ let cl_fixit_addreturn: () -> String = {
1010
"foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a closure expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
1111
}
1212

13+
func ff_fixit_addreturn_ifdecl() -> String {
14+
#if true
15+
print("entering ff_fixit_addreturn_ifdecl()")
16+
"foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a function expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
17+
#endif
18+
}
1319

20+
let cl_fixit_addreturn_ifdecl: () -> String = {
21+
#if true
22+
print("entering cl_fixit_addreturn_ifdecl()")
23+
"foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a closure expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
24+
#endif
25+
}

‎test/Parse/omit_return_fail.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,15 @@ func badIs<T>(_ value: Any, anInstanceOf type: T.Type) -> Bool {
77
func foo() -> Int {
88
return // expected-error {{non-void function should return a value}}
99
}
10+
11+
func badIs_ifdecl<T>(_ value: Any, anInstanceOf type: T.Type) -> Bool {
12+
#if true
13+
value is type // expected-error {{cannot find type 'type' in scope}}
14+
#endif
15+
}
16+
17+
func foo_ifdecl() -> Int {
18+
#if true
19+
return // expected-error {{non-void function should return a value}}
20+
#endif
21+
}

0 commit comments

Comments
 (0)