-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Allow return to be omitted from single expression functions. #23251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
nate-chandler
merged 33 commits into
swiftlang:master
from
nate-chandler:nate/omit-return
Apr 25, 2019
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
74b462b
WIP: Allow return to be omitted from single expression functions.
nate-chandler 64d4e30
Used a constraint system, not typeCheckExpression.
nate-chandler 018ba70
Just revert return insertion if inappropriate.
nate-chandler 2276e2a
Just reuse old body when reverting return insertion.
nate-chandler 08bbce3
Addressed issues from PR review.
nate-chandler c7f502e
Tested implicit return of most exprs from free functions.
nate-chandler ac7a76c
Don't insert return during code completion.
nate-chandler aa10bfa
Placed prechecked expressions back in body.
nate-chandler 72c6651
Only remove return for a unique viable solution.
nate-chandler 16cfada
Expected errors around implicitly returning #line.
nate-chandler 5262e19
Avoided double typechecking when possible.
nate-chandler 1bb7f29
Added comments explaining approach.
nate-chandler 25bbef1
Visited single expression of AbstractFunctionDecl.
nate-chandler 6aa8bfd
Corrected autoclosure handling.
nate-chandler 4f269fc
Added diagnostic for dangling expression at end.
nate-chandler 2cc8985
Added tests to cover autoclosure discriminators.
nate-chandler 859ffdc
Used expected-error to test fixit.
nate-chandler 717e5e4
Added PlaygroundTransform tests on implicit return.
nate-chandler 92998f7
Don't attempt to apply solution from salvage.
nate-chandler 0143ce2
Implicitly convert single exprs from uninhabited.
nate-chandler 5c1537f
Tested implicit non/conversion from uninhabited.
nate-chandler 0b528ae
Don't add an implicit () after return for initializers.
nate-chandler c21678d
Corrected tests by removing implicit returns.
nate-chandler 1071d97
Replaced superseded test.
nate-chandler 3139d3e
Tweaked remaining failing tests.
nate-chandler cca301a
Fixed up return omission test cases for Linux.
nate-chandler 448e04e
[ConstraintLocator] Add a special locator element to denote contextua…
xedin ca6cf0c
[ConstraintSystem] Use special locator for expr representing return o…
xedin b2ad562
Eliminated conversion from uninhabited.
nate-chandler a09a964
Don't implicitly return assignments.
nate-chandler 39bbce1
Reverted unneeded test change.
nate-chandler 843ac0b
Removed old comment.
nate-chandler f8ef213
Corrected off-by-one error.
nate-chandler File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,19 +42,37 @@ static void diagnoseMissingReturn(const UnreachableInst *UI, | |
SILLocation FLoc = F->getLocation(); | ||
|
||
Type ResTy; | ||
BraceStmt *BS; | ||
|
||
if (auto *FD = FLoc.getAsASTNode<FuncDecl>()) { | ||
ResTy = FD->getResultInterfaceType(); | ||
BS = FD->getBody(/*canSynthesize=*/false); | ||
} else if (auto *CD = FLoc.getAsASTNode<ConstructorDecl>()) { | ||
ResTy = CD->getResultInterfaceType(); | ||
BS = FD->getBody(); | ||
} else if (auto *CE = FLoc.getAsASTNode<ClosureExpr>()) { | ||
ResTy = CE->getResultType(); | ||
BS = CE->getBody(); | ||
} else { | ||
llvm_unreachable("unhandled case in MissingReturn"); | ||
} | ||
|
||
SILLocation L = UI->getLoc(); | ||
assert(L && ResTy); | ||
auto numElements = BS->getNumElements(); | ||
if (numElements > 0) { | ||
auto element = BS->getElement(numElements - 1); | ||
if (auto expr = element.dyn_cast<Expr *>()) { | ||
if (expr->getType()->getCanonicalType() == ResTy->getCanonicalType()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TypeBase::isEqual() is a shortcut for comparing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PR with a fix at #24546 . |
||
Context.Diags.diagnose( | ||
expr->getStartLoc(), | ||
diag::missing_return_last_expr, ResTy, | ||
FLoc.isASTNode<ClosureExpr>() ? 1 : 0) | ||
.fixItInsert(expr->getStartLoc(), "return "); | ||
return; | ||
} | ||
} | ||
} | ||
auto diagID = F->isNoReturnFunction() ? diag::missing_never_call | ||
: diag::missing_return; | ||
diagnose(Context, | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFD is an AbstractFunctionDecl so it will never have a kind that is Var or Subscript. In general we prefer to use
isa<>
instead of checking the kind directlyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR with a fix at #24546 .