Skip to content
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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

AnthonyLatsis
Copy link
Collaborator

Fixes #76362, fixes #76746, fixes #76747.

These are technically source breaking, but the cases in which they break source are quite niche.

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test Linux

Copy link
Contributor

@xedin xedin left a 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.

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test Linux

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`.
@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test Linux

@AnthonyLatsis
Copy link
Collaborator Author

@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);
Copy link
Contributor

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())) {
Copy link
Contributor

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

Comment on lines +921 to +923
if (SmallVector<ReturnStmt *> returnStmts;
func->getBody()->getExplicitReturnStmts(ctx, returnStmts),
!returnStmts.empty()) {
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants