Skip to content

Commit

Permalink
Warn about indentation of returned expr in single expression closure.
Browse files Browse the repository at this point in the history
If the returned expression has the same indentation as the "return"
keyword, warn. This warning already existed but wasn't happening
for single-expression closures. Move emission of the warning from Sema
to Parse.

<rdar://problem/16798323>
  • Loading branch information
cwillmor committed Nov 7, 2015
1 parent 6a0527e commit e7bdaea
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 40 deletions.
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,11 @@ ERROR(expected_close_after_else,stmt_parsing,none,
// Return Statement
ERROR(expected_expr_return,stmt_parsing,PointsToFirstBadToken,
"expected expression in 'return' statement", ())
WARNING(unindented_code_after_return,tce_sema,none,
"expression following 'return' is treated as an argument of "
"the 'return'", ())
NOTE(indent_expression_to_silence,tce_sema,none,
"indent the expression to silence this warning", ())

// Throw Statement
ERROR(expected_expr_throw,stmt_parsing,PointsToFirstBadToken,
Expand Down
6 changes: 0 additions & 6 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1881,12 +1881,6 @@ WARNING(store_in_willset,tce_sema,none,
"attempting to store to property %0 within its own willSet, which is "
"about to be overwritten by the new value", (Identifier))

WARNING(unindented_code_after_return,tce_sema,none,
"expression following 'return' is treated as an argument of "
"the 'return'", ())
NOTE(indent_expression_to_silence,tce_sema,none,
"indent the expression to silence this warning", ())

ERROR(value_of_module_type,tce_sema,none,
"expected module member name after module name", ())

Expand Down
8 changes: 8 additions & 0 deletions lib/Parse/ParseStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,14 @@ ParserResult<Stmt> Parser::parseStmtReturn(SourceLoc tryLoc) {
!isStartOfStmt() && !isStartOfDecl()) {
SourceLoc ExprLoc = Tok.getLoc();

// Issue a warning when the returned expression is on a different line than
// the return keyword, but both have the same indentation.
if (SourceMgr.getLineAndColumn(ReturnLoc).second ==
SourceMgr.getLineAndColumn(ExprLoc).second) {
diagnose(ExprLoc, diag::unindented_code_after_return);
diagnose(ExprLoc, diag::indent_expression_to_silence);
}

ParserResult<Expr> Result = parseExpr(diag::expected_expr_return);
if (Result.isNull()) {
// Create an ErrorExpr to tell the type checker that this return
Expand Down
34 changes: 0 additions & 34 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,39 +71,6 @@ static void diagSelfAssignment(TypeChecker &TC, const Expr *E) {
}


/// Issue a warning on code where a returned expression is on a different line
/// than the return keyword, but both have the same indentation.
///
/// \code
/// ...
/// return
/// foo()
/// \endcode
static void diagUnreachableCode(TypeChecker &TC, const Stmt *S) {
auto *RS = dyn_cast<ReturnStmt>(S);
if (!RS)
return;
if (!RS->hasResult())
return;

auto RetExpr = RS->getResult();
auto RSLoc = RS->getStartLoc();
auto RetExprLoc = RetExpr->getStartLoc();
// FIXME: Expose getColumnNumber() in LLVM SourceMgr to make this check
// cheaper.
if (RSLoc.isInvalid() || RetExprLoc.isInvalid() || (RSLoc == RetExprLoc))
return;
SourceManager &SM = TC.Context.SourceMgr;
if (SM.getLineAndColumn(RSLoc).second ==
SM.getLineAndColumn(RetExprLoc).second) {
TC.diagnose(RetExpr->getStartLoc(), diag::unindented_code_after_return);
TC.diagnose(RetExpr->getStartLoc(), diag::indent_expression_to_silence);
return;
}
return;
}


/// Diagnose syntactic restrictions of expressions.
///
/// - Module values may only occur as part of qualification.
Expand Down Expand Up @@ -1223,7 +1190,6 @@ void swift::performSyntacticExprDiagnostics(TypeChecker &TC, const Expr *E,

void swift::performStmtDiagnostics(TypeChecker &TC, const Stmt *S) {
TC.checkUnsupportedProtocolType(const_cast<Stmt *>(S));
return diagUnreachableCode(TC, S);
}

//===--------------------------------------------------------------------===//
Expand Down
7 changes: 7 additions & 0 deletions test/Sema/diag_unreachable_after_return.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ func unreachable_returns_Int() {
f_returns_Int() // expected-warning {{expression following 'return' is treated as an argument of the 'return'}} // expected-note{{indent the expression to silence this warning}}
}

func f_closure_returns_void() {
_ = {
return
f_closure_returns_void() // expected-warning {{expression following 'return' is treated as an argument of the 'return'}} // expected-note{{indent the expression to silence this warning}}
}
}

// Do not warn when the indentation is differnt.
func reachable_returns_void() {
return
Expand Down

0 comments on commit e7bdaea

Please sign in to comment.