diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index b3c00f731bbc8..f73ed4112a91d 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -928,13 +928,7 @@ bool TypeChecker::typeCheckForEachPreamble(DeclContext *dc, ForEachStmt *stmt, } bool TypeChecker::typeCheckCondition(Expr *&expr, DeclContext *dc) { - // If this expression is already typechecked and has type Bool, then just - // re-typecheck it. - if (expr->getType() && expr->getType()->isBool()) { - auto resultTy = - TypeChecker::typeCheckExpression(expr, dc); - return !resultTy; - } + ASSERT(!expr->getType() && "the bool condition is already type checked"); auto *boolDecl = dc->getASTContext().getBoolDecl(); if (!boolDecl) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 7259004ba1de6..1ce640845e5f6 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2727,6 +2727,18 @@ NamingPatternRequest::evaluate(Evaluator &evaluator, VarDecl *VD) const { if (!namingPattern) { if (auto parentStmt = VD->getRecursiveParentPatternStmt()) { + // We have a parent stmt. This should only ever be the case for completion + // or lazy type-checking, regular type-checking should go through the + // StmtChecker and assign types before querying this, otherwise we could + // end up double-type-checking. + // + // FIXME: We ought to be able to enable the following assert once we've + // fixed cases where we currently allow forward references to variables to + // kick interface type requests + // (https://github.com/swiftlang/swift/pull/85141). + // ASSERT(Context.SourceMgr.hasIDEInspectionTargetBuffer() || + // Context.TypeCheckerOpts.EnableLazyTypecheck); + // Try type checking parent control statement. if (auto condStmt = dyn_cast(parentStmt)) { // The VarDecl is defined inside a condition of a `if` or `while` stmt. @@ -2752,18 +2764,20 @@ NamingPatternRequest::evaluate(Evaluator &evaluator, VarDecl *VD) const { assert(foundVarDecl && "VarDecl not declared in its parent?"); (void) foundVarDecl; } else { - // We have some other parent stmt. Type check it completely. + // We have some other statement, e.g a switch or some kind of loop. We + // need to type-check it to get the type of the bound variable. We + // generally want to skip type-checking any BraceStmts, the only + // exception being do-catch bodies since we need to compute the thrown + // error type for catch clauses. + // FIXME: Rather than going through `typeCheckASTNode` and trying to + // exclude type-checker work, we ought to do more granular requests. + auto braceCheck = BraceStmtChecking::OnlyDoCatchBody; + if (auto CS = dyn_cast(parentStmt)) parentStmt = CS->getParentStmt(); - bool LeaveBodyUnchecked = true; - // type-checking 'catch' patterns depends on the type checked body. - if (isa(parentStmt)) - LeaveBodyUnchecked = false; - ASTNode node(parentStmt); - TypeChecker::typeCheckASTNode(node, VD->getDeclContext(), - LeaveBodyUnchecked); + TypeChecker::typeCheckASTNode(node, VD->getDeclContext(), braceCheck); } namingPattern = VD->getCanonicalVarDecl()->NamingPattern; } diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 6f1acdeb1f767..c9a03f2f22937 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -865,7 +865,6 @@ static bool typeCheckHasSymbolStmtConditionElement(StmtConditionElement &elt, static bool typeCheckBooleanStmtConditionElement(StmtConditionElement &elt, DeclContext *dc) { Expr *E = elt.getBoolean(); - assert(!E->getType() && "the bool condition is already type checked"); bool hadError = TypeChecker::typeCheckCondition(E, dc); elt.setBoolean(E); return hadError; @@ -1042,9 +1041,9 @@ class StmtChecker : public StmtVisitor { /// DC - This is the current DeclContext. DeclContext *DC; - /// Skip type checking any elements inside 'BraceStmt', also this is - /// propagated to ConstraintSystem. - bool LeaveBraceStmtBodyUnchecked = false; + /// The BraceStmts that should be type-checked. This should always be `All`, + /// unless we're lazy type-checking for e.g completion. + BraceStmtChecking BraceChecking = BraceStmtChecking::All; ASTContext &getASTContext() const { return Ctx; }; @@ -1503,7 +1502,7 @@ class StmtChecker : public StmtVisitor { // FIXME: This is a hack to avoid cycles through NamingPatternRequest when // doing lazy type-checking, we ought to fix the request to be granular in // the type-checking work it kicks. - bool skipWhere = LeaveBraceStmtBodyUnchecked && + bool skipWhere = (BraceChecking != BraceStmtChecking::All) && Ctx.SourceMgr.hasIDEInspectionTargetBuffer(); TypeChecker::typeCheckForEachPreamble(DC, S, skipWhere); @@ -1738,11 +1737,19 @@ class StmtChecker : public StmtVisitor { auto sourceFile = DC->getParentSourceFile(); checkLabeledStmtShadowing(getASTContext(), sourceFile, S); - // Type-check the 'do' body. Type failures in here will generally - // not cause type failures in the 'catch' clauses. - Stmt *newBody = S->getBody(); - typeCheckStmt(newBody); - S->setBody(newBody); + { + // If we have do-catch body checking enabled, make sure we visit the + // BraceStmt. + std::optional> braceChecking; + if (BraceChecking == BraceStmtChecking::OnlyDoCatchBody) + braceChecking.emplace(BraceChecking, BraceStmtChecking::All); + + // Type-check the 'do' body. Type failures in here will generally + // not cause type failures in the 'catch' clauses. + Stmt *newBody = S->getBody(); + typeCheckStmt(newBody); + S->setBody(newBody); + } // Do-catch statements always limit exhaustivity checks. bool limitExhaustivityChecks = true; @@ -2192,7 +2199,7 @@ void StmtChecker::typeCheckASTNode(ASTNode &node) { } Stmt *StmtChecker::visitBraceStmt(BraceStmt *BS) { - if (LeaveBraceStmtBodyUnchecked) + if (BraceChecking != BraceStmtChecking::All) return BS; // Diagnose defer statement being last one in block (only if @@ -2218,12 +2225,12 @@ Stmt *StmtChecker::visitBraceStmt(BraceStmt *BS) { } void TypeChecker::typeCheckASTNode(ASTNode &node, DeclContext *DC, - bool LeaveBodyUnchecked) { + BraceStmtChecking braceChecking) { StmtChecker stmtChecker(DC); // FIXME: 'ActiveLabeledStmts', etc. in StmtChecker are not // populated. Since they don't affect "type checking", it's doesn't cause // any issue for now. But it should be populated nonetheless. - stmtChecker.LeaveBraceStmtBodyUnchecked = LeaveBodyUnchecked; + stmtChecker.BraceChecking = braceChecking; stmtChecker.typeCheckASTNode(node); } @@ -2769,7 +2776,7 @@ bool TypeCheckASTNodeAtLocRequest::evaluate( } } - TypeChecker::typeCheckASTNode(finder.getRef(), DC, /*LeaveBodyUnchecked=*/false); + TypeChecker::typeCheckASTNode(finder.getRef(), DC); return false; } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index d609cc1725568..cfa7d9f982814 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -362,6 +362,18 @@ enum class CheckedCastContextKind { Coercion, }; +/// Determines which BraceStmts should be type-checked. +enum class BraceStmtChecking { + /// Type-check all BraceStmts. This should always be the case for regular + /// type-checking. + All, + + /// Only type-check the body of a do-catch statement, not including catch + /// clauses. This is necessary for completion where we still need the caught + /// error type to be computed. + OnlyDoCatchBody, +}; + namespace TypeChecker { // DANGER: callers must verify that elementType satisfies the requirements of // the Wrapped generic parameter, as this function will not do so! @@ -525,7 +537,7 @@ bool typeCheckStmtConditionElement(StmtConditionElement &elt, bool &isFalsable, ConcreteDeclRef getReferencedDeclForHasSymbolCondition(Expr *E); void typeCheckASTNode(ASTNode &node, DeclContext *DC, - bool LeaveBodyUnchecked = false); + BraceStmtChecking braceChecking = BraceStmtChecking::All); /// Try to apply the result builder transform of the given builder type /// to the body of the function. diff --git a/test/IDE/complete_exception.swift b/test/IDE/complete_exception.swift index 30b9ab0d1bd46..dba660268bc0f 100644 --- a/test/IDE/complete_exception.swift +++ b/test/IDE/complete_exception.swift @@ -189,3 +189,19 @@ func test016() { i.#^INSIDE_CATCH_TYPEDERR_DOT?check=INT_DOT^# } } + +// https://github.com/swiftlang/swift/issues/85434 +func issue85434() throws { + func foo() throws(Error4) {} + do { + try foo() + } catch let error { + switch error { + case .#^SWITCH_INSIDE_CATCH^# + // SWITCH_INSIDE_CATCH-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: E1[#Error4#]; name=E1 + // SWITCH_INSIDE_CATCH-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: E2({#Int32#})[#Error4#]; name=E2() + default: + throw error + } + } +} diff --git a/validation-test/IDE/crashers_fixed/ConstraintWalker-walkToExprPost-303c08.swift b/validation-test/IDE/crashers_fixed/ConstraintWalker-walkToExprPost-303c08.swift new file mode 100644 index 0000000000000..a54fc4e237dca --- /dev/null +++ b/validation-test/IDE/crashers_fixed/ConstraintWalker-walkToExprPost-303c08.swift @@ -0,0 +1,12 @@ +// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s +// https://github.com/swiftlang/swift/issues/85434 +protocol MyError: Error {} +func oops() { + do { + } catch let error as any MyError { + switch error.httpStatus { + case .#^COMPLETE^# + default: throw error + } + } +}