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

Allow return to be omitted from single expression functions. #23251

Merged
merged 33 commits into from
Apr 25, 2019

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Mar 12, 2019

Enables return to be omitted from function bodies which consist of only a single expression.

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

1 similar comment
@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler nate-chandler changed the title WIP: Allow return to be omitted from single expression functions. [WIP] Allow return to be omitted from single expression functions. Mar 13, 2019
@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

lib/AST/ASTWalker.cpp Outdated Show resolved Hide resolved
lib/AST/Decl.cpp Outdated Show resolved Hide resolved
lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved
lib/Parse/ParseDecl.cpp Show resolved Hide resolved
lib/Parse/ParseDecl.cpp Outdated Show resolved Hide resolved
test/Parse/omit_return.swift Show resolved Hide resolved
lib/Sema/TypeCheckStmt.cpp Outdated Show resolved Hide resolved
lib/Sema/TypeCheckStmt.cpp Outdated Show resolved Hide resolved
lib/Sema/TypeCheckStmt.cpp Outdated Show resolved Hide resolved
lib/Sema/TypeCheckStmt.cpp Outdated Show resolved Hide resolved
@jrose-apple
Copy link
Contributor

Forgot the summary at the top of "This looks pretty good; I'm glad it doesn't seem to take too many changes; good job listing out test cases."

Some other things to check:

  • code completion (as you mentioned)
  • what these look like when using SwiftSyntax

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

2 similar comments
@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler nate-chandler changed the title [WIP] Allow return to be omitted from single expression functions. Allow return to be omitted from single expression functions. Mar 21, 2019
@nate-chandler nate-chandler marked this pull request as ready for review March 21, 2019 22:10
@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 0a5e8f7

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - 0a5e8f7

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - a7daf25

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - a7daf25

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - f0ef9e3

@nate-chandler
Copy link
Contributor Author

@swift-ci please test source compatibility

nate-chandler and others added 17 commits April 24, 2019 09:59
Match the behavior of typeCheckExpressionImpl which simply calls solve
and applySolution but never salvage.
When type-checking a return statement's result, pass a new
ContextualTypePurpose when that return statement appears in a function
with a single expression.  When solving the corresponding constraint
system, the conversion constraint will have a new ConstraintKind.  When
matching types, check whether the constraint kind is this new kind,
meaning that the constraint is between a function's single expression
and the function's result.  If it is, allow a conversion from
an uninhabited type (the expression's type) to anything (the function's
result type) by adding an uninhabited upcast restriction to the vector
of conversions.  Finally, at coercion time, upon encountering this
restriction, call coerceUninhabited, replacing the original expression
with an UninhabitedUpcastExpr.  Finally, at SILGen time, treat this
UninhabitedUpcastExpr as a ForcedCheckedCastExpr.

Eliminates the bare ConstraintSystem usage from
typeCheckFunctionBodyUntil, ensuring that the same code path is followed
for all function bodies.
Modified so that single-expression implicit return does not throw off
tests.
Added objc_interop requirement to tests which require it and moved one
test into file stating that requirement.
Corrected a number of diagnostic regressions.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - e30511bdb8d8b4f0adc1b75e0db66992f4e25a74

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - e30511bdb8d8b4f0adc1b75e0db66992f4e25a74

@nate-chandler
Copy link
Contributor Author

@swift-ci Please clean smoke test OS X platform

@nate-chandler
Copy link
Contributor Author

@swift-ci please clean smoke test OS X platform

@nate-chandler
Copy link
Contributor Author

@swift-ci please clean test OS X platform

@nate-chandler nate-chandler merged commit 155a155 into apple:master Apr 25, 2019
@compnerd
Copy link
Collaborator

@nate-chandler this broke the Windows test suite!

// RUN: cp %s %t/main.swift
// RUN: %target-build-swift -Xfrontend -playground -Xfrontend -disable-playground-transform -o %t/main %t/main.swift
// RUN: %target-codesign %t/main
// RUN: ! %target-run %t/main --crash 2>&1 | %FileCheck -check-prefix=CRASH-CHECK %s
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use not.py next time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we prefer LLVM's not --crash.

if (numElements > 0) {
auto element = BS->getElement(numElements - 1);
if (auto expr = element.dyn_cast<Expr *>()) {
if (expr->getType()->getCanonicalType() == ResTy->getCanonicalType()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeBase::isEqual() is a shortcut for comparing getCanonicalType()s

Copy link
Contributor Author

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 .

auto Element = BS->getElement(0);
if (auto *stmt = Element.dyn_cast<Stmt *>()) {
auto kind = AFD->getKind();
if (kind == DeclKind::Var || kind == DeclKind::Subscript ||
Copy link
Member

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 directly

Copy link
Contributor Author

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 .

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

Successfully merging this pull request may close these issues.

None yet

10 participants