-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Sema: Small fixes for result builder inference #76749
base: main
Are you sure you want to change the base?
Conversation
@swift-ci please smoke test Linux |
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.
LGTM! I second Hamish's comment about BraceHasReturnRequest
it would be good to do at least that here.
37c326a
to
66d4cd1
Compare
@swift-ci please smoke test Linux |
…look at getter, not storage
A callback enables the caller to control whether to abort the walk. This is handy for `BraceHasExplicitReturnStmtRequest`.
… `BraceStmt` Also rename it to `getExplicitReturnStmts` for clarity and have it take a `SmallVector` out parameter instead as a small optimization and to discourage use of this new method as an alternative to `BraceStmt::hasExplicitReturnStatement`.
66d4cd1
to
02665c5
Compare
@swift-ci please smoke test Linux |
@swift-ci please test source compatibility release |
@@ -468,8 +468,7 @@ void typeCheckASTNode(ASTNode &node, DeclContext *DC, | |||
std::optional<BraceStmt *> applyResultBuilderBodyTransform(FuncDecl *func, | |||
Type builderType); | |||
|
|||
/// Find the return statements within the body of the given function. | |||
std::vector<ReturnStmt *> findReturnStatements(AnyFunctionRef fn); | |||
bool typeCheckClosureBody(ClosureExpr *closure); |
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.
Bad rebase? This function doesn't exist anymore
@@ -1126,8 +1124,7 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType, | |||
// not apply the result builder transform if it contained an explicit return. | |||
// To maintain source compatibility, we still need to check for HasReturnStmt. | |||
// https://github.com/apple/swift/issues/64332. | |||
if (evaluateOrDefault(getASTContext().evaluator, | |||
BraceHasReturnRequest{fn.getBody()}, false)) { | |||
if (fn.getBody()->hasExplicitReturnStmt(getASTContext())) { |
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.
One advantage of sticking the member on the AnyFunctionRef is you wouldn't have to pass along the ASTContext, but I don't feel too strongly about it
if (SmallVector<ReturnStmt *> returnStmts; | ||
func->getBody()->getExplicitReturnStmts(ctx, returnStmts), | ||
!returnStmts.empty()) { |
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.
Cute, but I would rather just bind returnStmts
outside the if
, up to you though
Fixes #76362, fixes #76746, fixes #76747.
These are technically source breaking, but the cases in which they break source are quite niche.