diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index d95675b40b24b..b947c44dcc8d8 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -959,11 +959,8 @@ AnnotatedInsertionPoint ConditionalClausePatternUseScope::expandAScopeThatCreatesANewInsertionPoint( ScopeCreator &scopeCreator) { auto *initializer = sec.getInitializer(); - if (!isa(initializer)) { - scopeCreator - .constructExpandAndInsert( - this, initializer); - } + scopeCreator.constructExpandAndInsert( + this, initializer); return {this, "Succeeding code must be in scope of conditional clause pattern bindings"}; diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 30abd5403f3c9..48ceec289c4ac 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1322,7 +1322,18 @@ namespace { } virtual Type visitErrorExpr(ErrorExpr *E) { - return recordInvalidNode(E); + auto ty = recordInvalidNode(E); + // If we have an original expression, introduce a conversion to the hole + // type of the ErrorExpr. This avoids unnecessary diagnostics for the + // original expression in cases where the ErrorExpr could have provided + // contextual info, while also still allowing the original expression to + // be solved without holes being introduced prematurely, allowing e.g + // cursor info to work correctly. + if (auto *orig = E->getOriginalExpr()) { + CS.addConstraint(ConstraintKind::Conversion, CS.getType(orig), ty, + CS.getConstraintLocator(E)); + } + return ty; } virtual Type visitCodeCompletionExpr(CodeCompletionExpr *E) { @@ -3156,6 +3167,16 @@ namespace { } case PatternKind::Expr: { + // Make sure we invalidate any nested VarDecls early since generating + // constraints for a `where` clause may happen before we've generated + // constraints for the ExprPattern. We'll record a fix when visiting + // the UnresolvedPatternExpr. + // FIXME: We ought to use a conjunction for switch cases, then we + // wouldn't need this logic. + auto *EP = cast(pattern); + EP->getSubExpr()->forEachUnresolvedVariable([&](VarDecl *VD) { + CS.setType(VD, ErrorType::get(CS.getASTContext())); + }); // We generate constraints for ExprPatterns in a separate pass. For // now, just create a type variable. return setType(CS.createTypeVariable(CS.getConstraintLocator(locator), diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index eb00690d08d73..9e41c972bdab3 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -5582,6 +5582,10 @@ bool ConstraintSystem::repairFailures( if (!anchor) return false; + // If we have an ErrorExpr anchor, we don't need to do any more fixes. + if (isExpr(anchor)) + return true; + // This could be: // - `InOutExpr` used with r-value e.g. `foo(&x)` where `x` is a `let`. // - `ForceValueExpr` e.g. `foo.bar! = 42` where `bar` or `foo` are diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index ddea97a1087c6..9584ec27111e7 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -1091,12 +1091,14 @@ class StmtChecker : public StmtVisitor { auto &eval = getASTContext().evaluator; auto *S = evaluateOrDefault(eval, PreCheckReturnStmtRequest{RS, DC}, nullptr); + if (!S) + return nullptr; - // We do a cast here as it may have been turned into a FailStmt. We should - // return that without doing anything else. - RS = dyn_cast_or_null(S); + // We do a cast here as we may now have a different stmt (e.g a FailStmt, or + // a BraceStmt for error cases). If so, recurse into the visitor. + RS = dyn_cast(S); if (!RS) - return S; + return visit(S); auto TheFunc = AnyFunctionRef::fromDeclContext(DC); assert(TheFunc && "Should have bailed from pre-check if this is None"); @@ -1785,16 +1787,31 @@ Stmt *PreCheckReturnStmtRequest::evaluate(Evaluator &evaluator, ReturnStmt *RS, auto &ctx = DC->getASTContext(); auto fn = AnyFunctionRef::fromDeclContext(DC); + auto errorResult = [&]() -> Stmt * { + // We don't need any recovery if there's no result, just bail. + if (!RS->hasResult()) + return nullptr; + + // If we have a result, make sure it's preserved, insert an implicit brace + // with a wrapping error expression. This ensures we can still do semantic + // functionality, and avoids downstream crashes where we expect the + // expression to have been type-checked. + auto *result = RS->getResult(); + RS->setResult(nullptr); + auto *err = new (ctx) ErrorExpr(result->getSourceRange(), Type(), result); + return BraceStmt::createImplicit(ctx, {err}); + }; + // Not valid outside of a function. if (!fn) { ctx.Diags.diagnose(RS->getReturnLoc(), diag::return_invalid_outside_func); - return nullptr; + return errorResult(); } // If the return is in a defer, then it isn't valid either. if (isDefer(DC)) { ctx.Diags.diagnose(RS->getReturnLoc(), diag::jump_out_of_defer, "return"); - return nullptr; + return errorResult(); } // The rest of the checks only concern return statements with results. @@ -1815,8 +1832,7 @@ Stmt *PreCheckReturnStmtRequest::evaluate(Evaluator &evaluator, ReturnStmt *RS, if (!nilExpr) { ctx.Diags.diagnose(RS->getReturnLoc(), diag::return_init_non_nil) .highlight(E->getSourceRange()); - RS->setResult(nullptr); - return RS; + return errorResult(); } // "return nil" is only permitted in a failable initializer. @@ -1826,8 +1842,7 @@ Stmt *PreCheckReturnStmtRequest::evaluate(Evaluator &evaluator, ReturnStmt *RS, ctx.Diags .diagnose(ctor->getLoc(), diag::make_init_failable, ctor) .fixItInsertAfter(ctor->getLoc(), "?"); - RS->setResult(nullptr); - return RS; + return errorResult(); } // Replace the "return nil" with a new 'fail' statement. diff --git a/test/IDE/complete_return.swift b/test/IDE/complete_return.swift index 466b173deedbb..5e28a85b9ab2f 100644 --- a/test/IDE/complete_return.swift +++ b/test/IDE/complete_return.swift @@ -1,21 +1,4 @@ -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_VOID_1 | %FileCheck %s -check-prefix=RETURN_VOID_1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_INT_1 | %FileCheck %s -check-prefix=RETURN_INT_1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_INT_2 | %FileCheck %s -check-prefix=RETURN_INT_2 - -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TRY_RETURN_INT | %FileCheck %s -check-prefix=RETURN_INT_1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TRY_RETURN_VOID | %FileCheck %s -check-prefix=RETURN_VOID_1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR1 | %FileCheck %s -check-prefix=RETURN_TR1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR2 | %FileCheck %s -check-prefix=RETURN_TR2 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR3 | %FileCheck %s -check-prefix=RETURN_TR3 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR1_METHOD | %FileCheck %s -check-prefix=RETURN_TR1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR2_METHOD | %FileCheck %s -check-prefix=RETURN_TR2 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR3_METHOD | %FileCheck %s -check-prefix=RETURN_TR3 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR1_STATICMETHOD | %FileCheck %s -check-prefix=RETURN_TR1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR2_STATICMETHOD | %FileCheck %s -check-prefix=RETURN_TR2 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR3_STATICMETHOD | %FileCheck %s -check-prefix=RETURN_TR3 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR1_CLOSURE | %FileCheck %s -check-prefix=RETURN_TR1 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR2_CLOSURE | %FileCheck %s -check-prefix=RETURN_TR2 -// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_TR3_CLOSURE | %FileCheck %s -check-prefix=RETURN_TR3 +// RUN: %batch-code-completion struct FooStruct { var instanceVar : Int @@ -64,11 +47,11 @@ func testReturnInt2(_ fooObject: FooStruct) { } func testMisplacedTry() throws -> Int { - try return #^TRY_RETURN_INT^# + try return #^TRY_RETURN_INT?check=RETURN_INT_1^# } func testMisplacedTryVoid() throws { - try return #^TRY_RETURN_VOID^# + try return #^TRY_RETURN_VOID?check=RETURN_VOID_1^# } func testTR1() -> Int? { @@ -111,26 +94,26 @@ struct TestStruct { var i : Int var oi : Int? var fs : FooStruct - return #^RETURN_TR1_METHOD^# + return #^RETURN_TR1_METHOD?check=RETURN_TR1^# } func testTR2_method(_ g : Gen) -> Int? { - return g.#^RETURN_TR2_METHOD^# + return g.#^RETURN_TR2_METHOD?check=RETURN_TR2^# } func testTR3_method(_ g : Gen) -> Int? { - return g.IG.#^RETURN_TR3_METHOD^# + return g.IG.#^RETURN_TR3_METHOD?check=RETURN_TR3^# } static func testTR1_static() -> Int? { var i : Int var oi : Int? var fs : FooStruct - return #^RETURN_TR1_STATICMETHOD^# + return #^RETURN_TR1_STATICMETHOD?check=RETURN_TR1^# } static func testTR2_static(_ g : Gen) -> Int? { - return g.#^RETURN_TR2_STATICMETHOD^# + return g.#^RETURN_TR2_STATICMETHOD?check=RETURN_TR2^# } static func testTR3_static(_ g : Gen) -> Int? { - return g.IG.#^RETURN_TR3_STATICMETHOD^# + return g.IG.#^RETURN_TR3_STATICMETHOD?check=RETURN_TR3^# } } @@ -140,12 +123,26 @@ func testClosures(_ g: Gen) { var fs : FooStruct _ = { () -> Int? in - return #^RETURN_TR1_CLOSURE^# + return #^RETURN_TR1_CLOSURE?check=RETURN_TR1^# } _ = { () -> Int? in - return g.#^RETURN_TR2_CLOSURE^# + return g.#^RETURN_TR2_CLOSURE?check=RETURN_TR2^# } _ = { () -> Int? in - return g.IG.#^RETURN_TR3_CLOSURE^# + return g.IG.#^RETURN_TR3_CLOSURE?check=RETURN_TR3^# + } +} + +// Make sure we can do a completion in an out-of-place return +do { + return TestStruct.#^COMPLETE_IN_INVALID_RETURN^# + // COMPLETE_IN_INVALID_RETURN: Decl[StaticMethod]/CurrNominal: testTR1_static()[#Int?#]; name=testTR1_static() + // COMPLETE_IN_INVALID_RETURN: Decl[StaticMethod]/CurrNominal: testTR2_static({#(g): Gen#})[#Int?#]; name=testTR2_static(:) + // COMPLETE_IN_INVALID_RETURN: Decl[StaticMethod]/CurrNominal: testTR3_static({#(g): Gen#})[#Int?#]; name=testTR3_static(:) +} + +struct TestReturnInInit { + init() { + return TestStruct.#^COMPLETE_IN_INVALID_INIT_RETURN?check=COMPLETE_IN_INVALID_RETURN^# } } diff --git a/test/stmt/statements.swift b/test/stmt/statements.swift index 566599474bc91..4c604127b7903 100644 --- a/test/stmt/statements.swift +++ b/test/stmt/statements.swift @@ -179,6 +179,8 @@ return 42 // expected-error {{return invalid outside of a func}} return // expected-error {{return invalid outside of a func}} +return VoidReturn1() // expected-error {{return invalid outside of a func}} + func NonVoidReturn1() -> Int { _ = 0 return // expected-error {{non-void function should return a value}} diff --git a/validation-test/compiler_crashers/ConstraintSystem-getClosureType-aac79a.swift b/validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-aac79a.swift similarity index 81% rename from validation-test/compiler_crashers/ConstraintSystem-getClosureType-aac79a.swift rename to validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-aac79a.swift index 313b604bae0cc..806102beb1aa3 100644 --- a/validation-test/compiler_crashers/ConstraintSystem-getClosureType-aac79a.swift +++ b/validation-test/compiler_crashers_fixed/ConstraintSystem-getClosureType-aac79a.swift @@ -1,5 +1,5 @@ // {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::getClosureType(swift::ClosureExpr const*) const","signatureAssert":"Assertion failed: (result), function getClosureType"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s return { lazy var a = if .random() { return } } diff --git a/validation-test/compiler_crashers_fixed/ConstraintSystem-getTypeOfReferencePre-60045c.swift b/validation-test/compiler_crashers_fixed/ConstraintSystem-getTypeOfReferencePre-60045c.swift new file mode 100644 index 0000000000000..aece4ac77ecd1 --- /dev/null +++ b/validation-test/compiler_crashers_fixed/ConstraintSystem-getTypeOfReferencePre-60045c.swift @@ -0,0 +1,9 @@ +// {"kind":"typecheck","signature":"swift::constraints::ConstraintSystem::getTypeOfReferencePre(swift::constraints::OverloadChoice, swift::DeclContext*, swift::constraints::ConstraintLocatorBuilder, swift::constraints::PreparedOverloadBuilder*)","signatureAssert":"Assertion failed: (func->isOperator() && \"Lookup should only find operators\"), function getTypeOfReferencePre"} +// RUN: not %target-swift-frontend -typecheck %s +{ + switch if .random() else { + } + { + case let !a where a: + } +} diff --git a/validation-test/compiler_crashers_fixed/TypeVarRefCollector-walkToStmtPre-5c070f.swift b/validation-test/compiler_crashers_fixed/TypeVarRefCollector-walkToStmtPre-5c070f.swift new file mode 100644 index 0000000000000..a714f8547e0b9 --- /dev/null +++ b/validation-test/compiler_crashers_fixed/TypeVarRefCollector-walkToStmtPre-5c070f.swift @@ -0,0 +1,7 @@ +// {"kind":"typecheck","signature":"swift::constraints::TypeVarRefCollector::walkToStmtPre(swift::Stmt*)","signatureAssert":"Assertion failed: (result), function getClosureType"} +// RUN: not %target-swift-frontend -typecheck %s +{ + guard let a = (a if{ + return + } +) " diff --git a/validation-test/compiler_crashers_fixed/TypeVarRefCollector-walkToStmtPre-e56ccf.swift b/validation-test/compiler_crashers_fixed/TypeVarRefCollector-walkToStmtPre-e56ccf.swift new file mode 100644 index 0000000000000..2f88c27677f69 --- /dev/null +++ b/validation-test/compiler_crashers_fixed/TypeVarRefCollector-walkToStmtPre-e56ccf.swift @@ -0,0 +1,10 @@ +// {"kind":"typecheck","signature":"swift::constraints::TypeVarRefCollector::walkToStmtPre(swift::Stmt*)","signatureAssert":"Assertion failed: (result), function getClosureType"} +// RUN: not %target-swift-frontend -typecheck %s +{ + switch if <#expression#> { + return + } + { + case let !a where a: + } +}