From 6ee44f09b480a912a50b5b6dde93c6a9717e98f9 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Fri, 1 Sep 2023 14:32:14 +0100 Subject: [PATCH 1/2] Introduce `then` statements These allow multi-statement `if`/`switch` expression branches that can produce a value at the end by saying `then `. This is gated behind `-enable-experimental-feature ThenStatements` pending evolution discussion. --- include/swift/AST/DiagnosticsParse.def | 11 +- include/swift/AST/DiagnosticsSema.def | 8 +- include/swift/AST/Expr.h | 24 +- include/swift/AST/NameLookup.h | 1 + include/swift/AST/Stmt.h | 32 +++ include/swift/AST/StmtNodes.def | 1 + include/swift/AST/TokenKinds.def | 1 + include/swift/AST/TypeCheckRequests.h | 8 +- include/swift/Basic/Features.def | 3 + include/swift/IDE/CodeCompletionResult.h | 1 + include/swift/Parse/IDEInspectionCallbacks.h | 2 + include/swift/Parse/Parser.h | 15 +- include/swift/Parse/Token.h | 33 ++- include/swift/Sema/ConstraintLocator.h | 24 +- .../swift/Sema/ConstraintLocatorPathElts.def | 4 +- lib/AST/ASTDumper.cpp | 7 + lib/AST/ASTPrinter.cpp | 14 ++ lib/AST/ASTScopeCreation.cpp | 6 + lib/AST/ASTVerifier.cpp | 2 +- lib/AST/ASTWalker.cpp | 9 + lib/AST/Expr.cpp | 64 +++-- lib/AST/Stmt.cpp | 22 ++ lib/ASTGen/Sources/ASTGen/SourceFile.swift | 1 + lib/IDE/CodeCompletion.cpp | 23 +- lib/IDE/TypeContextInfo.cpp | 5 + lib/Parse/ParseExpr.cpp | 7 +- lib/Parse/ParseStmt.cpp | 125 ++++++++-- lib/Parse/Parser.cpp | 8 +- lib/SILGen/SILGenExpr.cpp | 2 +- lib/SILGen/SILGenStmt.cpp | 22 +- .../Mandatory/MoveOnlyDiagnostics.cpp | 1 + lib/Sema/BuilderTransform.cpp | 6 + lib/Sema/CSApply.cpp | 12 - lib/Sema/CSDiagnostics.cpp | 2 +- lib/Sema/CSGen.cpp | 4 +- lib/Sema/CSSimplify.cpp | 22 +- lib/Sema/CSSyntacticElement.cpp | 102 +++++--- lib/Sema/ConstraintLocator.cpp | 23 +- lib/Sema/ConstraintSystem.cpp | 8 +- lib/Sema/MiscDiagnostics.cpp | 21 +- lib/Sema/TypeCheckStmt.cpp | 99 +++++--- test/Constraints/then_stmt.swift | 109 +++++++++ test/IDE/complete_then_stmt.swift | 84 +++++++ test/stmt/print/then_stmt.swift | 18 ++ test/stmt/then_stmt.swift | 220 ++++++++++++++++++ test/stmt/then_stmt_disabled.swift | 11 + test/stmt/then_stmt_exec.swift | 32 +++ 47 files changed, 1063 insertions(+), 196 deletions(-) create mode 100644 test/Constraints/then_stmt.swift create mode 100644 test/IDE/complete_then_stmt.swift create mode 100644 test/stmt/print/then_stmt.swift create mode 100644 test/stmt/then_stmt.swift create mode 100644 test/stmt/then_stmt_disabled.swift create mode 100644 test/stmt/then_stmt_exec.swift diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def index 8f55dbb4180b5..4a404cc797611 100644 --- a/include/swift/AST/DiagnosticsParse.def +++ b/include/swift/AST/DiagnosticsParse.def @@ -1244,8 +1244,9 @@ ERROR(case_stmt_without_body,none, // 'try' on statements ERROR(try_on_stmt,none, "'try' cannot be used with '%0'", (StringRef)) -ERROR(try_on_return_throw_yield,none, - "'try' must be placed on the %select{returned|thrown|yielded}0 expression", +ERROR(try_must_come_after_stmt,none, + "'try' must be placed on the %select{returned|thrown|yielded|produced}0 " + "expression", (unsigned)) ERROR(try_on_var_let,none, "'try' must be placed on the initial value expression", ()) @@ -1399,6 +1400,12 @@ ERROR(expected_expr_after_each, none, "expected expression after 'each'", ()) ERROR(expected_expr_after_repeat, none, "expected expression after 'repeat'", ()) +ERROR(expected_expr_after_then,none, + "expected expression after 'then'", ()) + +WARNING(unindented_code_after_then,none, + "expression following 'then' is treated as an argument of " + "the 'then'", ()) WARNING(move_consume_final_spelling, none, "'_move' has been renamed to 'consume', and the '_move' spelling will be removed shortly", ()) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 5e33b14adbbd6..101252466951b 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1231,14 +1231,18 @@ ERROR(if_expr_must_be_syntactically_exhaustive,none, ERROR(single_value_stmt_branch_empty,none, "expected expression in branch of '%0' expression", (StmtKind)) -ERROR(single_value_stmt_branch_must_end_in_throw,none, - "non-expression branch of '%0' expression may only end with a 'throw'", +ERROR(single_value_stmt_branch_must_end_in_result,none, + "non-expression branch of '%0' expression may only end with a 'throw' " + "or 'then'", (StmtKind)) ERROR(cannot_jump_in_single_value_stmt,none, "cannot '%0' in '%1' when used as expression", (StmtKind, StmtKind)) ERROR(effect_marker_on_single_value_stmt,none, "'%0' may not be used on '%1' expression", (StringRef, StmtKind)) +ERROR(out_of_place_then_stmt,none, + "'then' may only appear as the last statement in an 'if' or 'switch' " + "expression", ()) ERROR(did_not_call_function_value,none, "function value was used as a property; add () to call it", diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index baf2b6167ba5d..96a73b7c7d35f 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -70,6 +70,7 @@ namespace swift { class CallExpr; class KeyPathExpr; class CaptureListExpr; + class ThenStmt; enum class ExprKind : uint8_t { #define EXPR(Id, Parent) Id, @@ -6125,10 +6126,10 @@ class SingleValueStmtExpr : public Expr { SingleValueStmtExpr(Stmt *S, DeclContext *DC) : Expr(ExprKind::SingleValueStmt, /*isImplicit*/ true), S(S), DC(DC) {} -public: /// Creates a new SingleValueStmtExpr wrapping a statement. static SingleValueStmtExpr *create(ASTContext &ctx, Stmt *S, DeclContext *DC); +public: /// Creates a new SingleValueStmtExpr wrapping a statement, and recursively /// attempts to wrap any branches of that statement that can become single /// value statement expressions. @@ -6144,6 +6145,16 @@ class SingleValueStmtExpr : public Expr { /// SingleValueStmtExpr. static SingleValueStmtExpr *tryDigOutSingleValueStmtExpr(Expr *E); + /// Retrieves a resulting ThenStmt from the given BraceStmt, or \c nullptr if + /// the brace does not have a resulting ThenStmt. + static ThenStmt *getThenStmtFrom(BraceStmt *BS); + + /// Whether the given BraceStmt has a result to be produced from a parent + /// SingleValueStmtExpr. + static bool hasResult(BraceStmt *BS) { + return getThenStmtFrom(BS); + } + /// Retrieve the wrapped statement. Stmt *getStmt() const { return S; } void setStmt(Stmt *newS) { S = newS; } @@ -6154,10 +6165,15 @@ class SingleValueStmtExpr : public Expr { /// Retrieve the complete set of branches for the underlying statement. ArrayRef getBranches(SmallVectorImpl &scratch) const; - /// Retrieve the single expression branches of the statement, excluding - /// branches that either have multiple expressions, or have statements. + /// Retrieve the resulting ThenStmts from each branch of the + /// SingleValueStmtExpr. + ArrayRef + getThenStmts(SmallVectorImpl &scratch) const; + + /// Retrieve the result expressions from each branch of the + /// SingleValueStmtExpr. ArrayRef - getSingleExprBranches(SmallVectorImpl &scratch) const; + getResultExprs(SmallVectorImpl &scratch) const; DeclContext *getDeclContext() const { return DC; } diff --git a/include/swift/AST/NameLookup.h b/include/swift/AST/NameLookup.h index c2009567b0621..14701ef819f7b 100644 --- a/include/swift/AST/NameLookup.h +++ b/include/swift/AST/NameLookup.h @@ -667,6 +667,7 @@ class FindLocalVal : public StmtVisitor { void visitFailStmt(FailStmt *) {} void visitReturnStmt(ReturnStmt *) {} void visitYieldStmt(YieldStmt *) {} + void visitThenStmt(ThenStmt *) {} void visitThrowStmt(ThrowStmt *) {} void visitDiscardStmt(DiscardStmt *) {} void visitPoundAssertStmt(PoundAssertStmt *) {} diff --git a/include/swift/AST/Stmt.h b/include/swift/AST/Stmt.h index 7a247964c5ac7..f62ff4bf34313 100644 --- a/include/swift/AST/Stmt.h +++ b/include/swift/AST/Stmt.h @@ -304,6 +304,38 @@ class YieldStmt final static bool classof(const Stmt *S) { return S->getKind() == StmtKind::Yield; } }; +/// The statement `then `. This is used within if/switch expressions to +/// indicate the value being produced by a given branch. +class ThenStmt : public Stmt { + SourceLoc ThenLoc; + Expr *Result; + + ThenStmt(SourceLoc thenLoc, Expr *result, bool isImplicit) + : Stmt(StmtKind::Then, isImplicit), ThenLoc(thenLoc), Result(result) { + assert(Result && "Must have non-null result"); + } + +public: + /// Create a new parsed ThenStmt. + static ThenStmt *createParsed(ASTContext &ctx, SourceLoc thenLoc, + Expr *result); + + /// Create an implicit ThenStmt. + /// + /// Note that such statements will be elided during the result builder + /// transform. + static ThenStmt *createImplicit(ASTContext &ctx, Expr *result); + + SourceLoc getThenLoc() const { return ThenLoc; } + + SourceRange getSourceRange() const; + + Expr *getResult() const { return Result; } + void setResult(Expr *e) { Result = e; } + + static bool classof(const Stmt *S) { return S->getKind() == StmtKind::Then; } +}; + /// DeferStmt - A 'defer' statement. This runs the substatement it contains /// when the enclosing scope is exited. /// diff --git a/include/swift/AST/StmtNodes.def b/include/swift/AST/StmtNodes.def index ec643587600e9..b35149ad7f437 100644 --- a/include/swift/AST/StmtNodes.def +++ b/include/swift/AST/StmtNodes.def @@ -46,6 +46,7 @@ STMT(Brace, Stmt) STMT(Return, Stmt) +STMT(Then, Stmt) STMT(Yield, Stmt) STMT(Defer, Stmt) ABSTRACT_STMT(Labeled, Stmt) diff --git a/include/swift/AST/TokenKinds.def b/include/swift/AST/TokenKinds.def index e929b1a18de57..5217cf89139a3 100644 --- a/include/swift/AST/TokenKinds.def +++ b/include/swift/AST/TokenKinds.def @@ -258,6 +258,7 @@ MISC(string_segment) MISC(string_interpolation_anchor) MISC(kw_yield) MISC(kw_discard) +MISC(kw_then) // The following tokens are irrelevant for swiftSyntax and thus only declared // in this .def file diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index e82811bb5718c..ae43a7fb4131e 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -3901,8 +3901,8 @@ class IsSingleValueStmtResult { /// The statement is an 'if' statement without an unconditional 'else'. NonExhaustiveIf, - /// There are no single-expression branches. - NoExpressionBranches, + /// There is no branch that produces a resulting value. + NoResult, /// There is an unhandled statement branch. This should only be the case /// for invalid AST. @@ -3957,8 +3957,8 @@ class IsSingleValueStmtResult { static IsSingleValueStmtResult nonExhaustiveIf() { return IsSingleValueStmtResult(Kind::NonExhaustiveIf); } - static IsSingleValueStmtResult noExpressionBranches() { - return IsSingleValueStmtResult(Kind::NoExpressionBranches); + static IsSingleValueStmtResult noResult() { + return IsSingleValueStmtResult(Kind::NoResult); } static IsSingleValueStmtResult unhandledStmt() { return IsSingleValueStmtResult(Kind::UnhandledStmt); diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def index 6b8524d9fb38c..4e1bee0f63aff 100644 --- a/include/swift/Basic/Features.def +++ b/include/swift/Basic/Features.def @@ -226,6 +226,9 @@ EXPERIMENTAL_FEATURE(GlobalConcurrency, false) /// "playground transform" is enabled. EXPERIMENTAL_FEATURE(PlaygroundExtendedCallbacks, true) +/// Enable 'then' statements. +EXPERIMENTAL_FEATURE(ThenStatements, false) + /// Enable the `@_rawLayout` attribute. EXPERIMENTAL_FEATURE(RawLayout, true) diff --git a/include/swift/IDE/CodeCompletionResult.h b/include/swift/IDE/CodeCompletionResult.h index b699e65dd04a8..3b67303aa1f8c 100644 --- a/include/swift/IDE/CodeCompletionResult.h +++ b/include/swift/IDE/CodeCompletionResult.h @@ -213,6 +213,7 @@ enum class CompletionKind : uint8_t { CallArg, LabeledTrailingClosure, ReturnStmtExpr, + ThenStmtExpr, YieldStmtExpr, ForEachSequence, diff --git a/include/swift/Parse/IDEInspectionCallbacks.h b/include/swift/Parse/IDEInspectionCallbacks.h index e3921e01fbae5..3b757b5ec46ee 100644 --- a/include/swift/Parse/IDEInspectionCallbacks.h +++ b/include/swift/Parse/IDEInspectionCallbacks.h @@ -235,6 +235,8 @@ class CodeCompletionCallbacks { virtual void completeReturnStmt(CodeCompletionExpr *E) {}; + virtual void completeThenStmt(CodeCompletionExpr *E) {}; + /// Complete a yield statement. A missing yield index means that the /// completion immediately follows the 'yield' keyword; it may be either /// an expression or a parenthesized expression list. A present yield diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h index 6089e20a19d34..418205071e354 100644 --- a/include/swift/Parse/Parser.h +++ b/include/swift/Parse/Parser.h @@ -602,6 +602,10 @@ class Parser { cast(CurDeclContext)->isCoroutine()); } + /// Whether the current token is the contextual keyword for a \c then + /// statement. + bool isContextualThenKeyword(bool preferExpr); + /// `discard self` is the only valid phrase, but we peek ahead for just any /// identifier after `discard` to determine if it's the statement. This helps /// us avoid interpreting `discard(self)` as the statement and not a call. @@ -641,7 +645,7 @@ class Parser { while (Tok.isNot(K..., tok::eof, tok::r_brace, tok::pound_endif, tok::pound_else, tok::pound_elseif, tok::code_complete) && - !isStartOfStmt() && + !isStartOfStmt(/*preferExpr*/ false) && !isStartOfSwiftDecl(/*allowPoundIfAttributes=*/true)) { skipSingle(); } @@ -1919,7 +1923,13 @@ class Parser { //===--------------------------------------------------------------------===// // Statement Parsing - bool isStartOfStmt(); + /// Whether we are at the start of a statement. + /// + /// \param preferExpr If either an expression or statement could be parsed and + /// this parameter is \c true, the function returns \c false such that an + /// expression can be parsed. + bool isStartOfStmt(bool preferExpr); + bool isTerminatorForBraceItemListKind(BraceItemListKind Kind, ArrayRef ParsedDecls); ParserResult parseStmt(); @@ -1928,6 +1938,7 @@ class Parser { ParserResult parseStmtContinue(); ParserResult parseStmtReturn(SourceLoc tryLoc); ParserResult parseStmtYield(SourceLoc tryLoc); + ParserResult parseStmtThen(SourceLoc tryLoc); ParserResult parseStmtThrow(SourceLoc tryLoc); ParserResult parseStmtDiscard(); ParserResult parseStmtDefer(); diff --git a/include/swift/Parse/Token.h b/include/swift/Parse/Token.h index 99e1da03d1a81..9080d07a7103a 100644 --- a/include/swift/Parse/Token.h +++ b/include/swift/Parse/Token.h @@ -108,7 +108,38 @@ class Token { bool isBinaryOperator() const { return Kind == tok::oper_binary_spaced || Kind == tok::oper_binary_unspaced; } - + + /// Checks whether the token is either a binary operator, or is a token that + /// acts like a binary operator (e.g infix '=', '?', '->'). + bool isBinaryOperatorLike() const { + if (isBinaryOperator()) + return true; + + switch (Kind) { + case tok::equal: + case tok::arrow: + case tok::question_infix: + return true; + default: + return false; + } + llvm_unreachable("Unhandled case in switch!"); + } + + /// Checks whether the token is either a postfix operator, or is a token that + /// acts like a postfix operator (e.g postfix '!' and '?'). + bool isPostfixOperatorLike() const { + switch (Kind) { + case tok::oper_postfix: + case tok::exclaim_postfix: + case tok::question_postfix: + return true; + default: + return false; + } + llvm_unreachable("Unhandled case in switch!"); + } + bool isAnyOperator() const { return isBinaryOperator() || Kind == tok::oper_postfix || Kind == tok::oper_prefix; diff --git a/include/swift/Sema/ConstraintLocator.h b/include/swift/Sema/ConstraintLocator.h index d3c9d01627b26..895c104ae3be3 100644 --- a/include/swift/Sema/ConstraintLocator.h +++ b/include/swift/Sema/ConstraintLocator.h @@ -103,8 +103,14 @@ enum class ConversionRestrictionKind; /// The kind of SingleValueStmtExpr branch the locator identifies. enum class SingleValueStmtBranchKind { - Regular, - InSingleExprClosure + /// Explicitly written 'then '. + Explicit, + + /// Implicitly written ''. + Implicit, + + /// Implicitly written '' in a single expr closure body. + ImplicitInSingleExprClosure }; /// Locates a given constraint within the expression being @@ -938,19 +944,19 @@ class LocatorPathElt::TernaryBranch final : public StoredIntegerElement<1> { } }; -/// The branch of a SingleValueStmtExpr. Note the stored index corresponds to -/// the expression branches, i.e it skips statement branch indices. -class LocatorPathElt::SingleValueStmtBranch final +/// A particular result of a ThenStmt at a given index in a SingleValueStmtExpr. +/// The stored index corresponds to the \c getResultExprs array. +class LocatorPathElt::SingleValueStmtResult final : public StoredIntegerElement<1> { public: - SingleValueStmtBranch(unsigned exprIdx) - : StoredIntegerElement(ConstraintLocator::SingleValueStmtBranch, + SingleValueStmtResult(unsigned exprIdx) + : StoredIntegerElement(ConstraintLocator::SingleValueStmtResult, exprIdx) {} - unsigned getExprBranchIndex() const { return getValue(); } + unsigned getIndex() const { return getValue(); } static bool classof(const LocatorPathElt *elt) { - return elt->getKind() == ConstraintLocator::SingleValueStmtBranch; + return elt->getKind() == ConstraintLocator::SingleValueStmtResult; } }; diff --git a/include/swift/Sema/ConstraintLocatorPathElts.def b/include/swift/Sema/ConstraintLocatorPathElts.def index c412b998a827a..82e6b3ab922f6 100644 --- a/include/swift/Sema/ConstraintLocatorPathElts.def +++ b/include/swift/Sema/ConstraintLocatorPathElts.def @@ -259,8 +259,8 @@ CUSTOM_LOCATOR_PATH_ELT(SyntacticElement) /// The element of the pattern binding declaration. CUSTOM_LOCATOR_PATH_ELT(PatternBindingElement) -/// A branch of a SingleValueStmtExpr. -CUSTOM_LOCATOR_PATH_ELT(SingleValueStmtBranch) +/// A particular result of a ThenStmt at a given index in a SingleValueStmtExpr. +CUSTOM_LOCATOR_PATH_ELT(SingleValueStmtResult) /// A declaration introduced by a pattern: name (i.e. `x`) or `_` ABSTRACT_LOCATOR_PATH_ELT(PatternDecl) diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index 191129e4d507a..80e458e546027 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -1624,6 +1624,13 @@ class PrintStmt : public StmtVisitor { PrintWithColorRAII(OS, ParenthesisColor) << ')'; } + void visitThenStmt(ThenStmt *S) { + printCommon(S, "then_stmt"); + OS << '\n'; + printRec(S->getResult()); + PrintWithColorRAII(OS, ParenthesisColor) << ')'; + } + void visitDeferStmt(DeferStmt *S) { printCommon(S, "defer_stmt") << '\n'; printRec(S->getTempDecl()); diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index a95b30e4e9f55..f699d042b6dc9 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3502,6 +3502,10 @@ static bool usesFeaturePlaygroundExtendedCallbacks(Decl *decl) { return false; } +static bool usesFeatureThenStatements(Decl *decl) { + return false; +} + static bool usesFeatureNewCxxMethodSafetyHeuristics(Decl *decl) { return decl->hasClangNode(); } @@ -5514,6 +5518,16 @@ void PrintAST::visitYieldStmt(YieldStmt *stmt) { if (parens) Printer << ")"; } +void PrintAST::visitThenStmt(ThenStmt *stmt) { + // For now, don't print implicit 'then' statements, since they can be + // present when the feature is disabled. + // TODO: Once we enable the feature, we can remove this. + if (!stmt->isImplicit()) + Printer.printKeyword("then", Options, " "); + + visit(stmt->getResult()); +} + void PrintAST::visitThrowStmt(ThrowStmt *stmt) { Printer << tok::kw_throw << " "; visit(stmt->getSubExpr()); diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index 52bb4419b9699..f80c2ad364682 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -437,6 +437,12 @@ class NodeAdder return p; } + ASTScopeImpl *visitThenStmt(ThenStmt *ts, ASTScopeImpl *p, + ScopeCreator &scopeCreator) { + visitExpr(ts->getResult(), p, scopeCreator); + return p; + } + ASTScopeImpl *visitDeferStmt(DeferStmt *ds, ASTScopeImpl *p, ScopeCreator &scopeCreator) { visitFuncDecl(ds->getTempDecl(), p, scopeCreator); diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp index 443282b031b29..575b724e8610f 100644 --- a/lib/AST/ASTVerifier.cpp +++ b/lib/AST/ASTVerifier.cpp @@ -1214,7 +1214,7 @@ class Verifier : public ASTWalker { void verifyChecked(SingleValueStmtExpr *E) { using Kind = IsSingleValueStmtResult::Kind; switch (E->getStmt()->mayProduceSingleValue(Ctx).getKind()) { - case Kind::NoExpressionBranches: + case Kind::NoResult: // These are allowed as long as the type is Void. checkSameType( E->getType(), Ctx.getVoidType(), diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index e1d3b1dc68d8c..8c4c3d2c8adf1 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -1772,6 +1772,15 @@ Stmt *Traversal::visitYieldStmt(YieldStmt *YS) { return YS; } +Stmt *Traversal::visitThenStmt(ThenStmt *TS) { + auto *E = doIt(TS->getResult()); + if (!E) + return nullptr; + + TS->setResult(E); + return TS; +} + Stmt *Traversal::visitDeferStmt(DeferStmt *DS) { if (doIt(DS->getTempDecl())) return nullptr; diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 94f627e9f337a..540f757f1ddd2 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -2492,27 +2492,29 @@ SingleValueStmtExpr *SingleValueStmtExpr::createWithWrappedBranches( if (!BS) continue; - auto *S = BS->getSingleActiveStatement(); - if (!S) - continue; - - if (mustBeExpr) { - // If this must be an expression, we can eagerly wrap any exhaustive if - // and switch branch. - if (auto *IS = dyn_cast(S)) { - if (!IS->isSyntacticallyExhaustive()) + if (auto *S = BS->getSingleActiveStatement()) { + if (mustBeExpr) { + // If this must be an expression, we can eagerly wrap any exhaustive if + // and switch branch. + if (auto *IS = dyn_cast(S)) { + if (!IS->isSyntacticallyExhaustive()) + continue; + } else if (!isa(S)) { + continue; + } + } else { + // Otherwise do the semantic checking to verify that we can wrap the + // branch. + if (!S->mayProduceSingleValue(ctx)) continue; - } else if (!isa(S)) { - continue; } - } else { - // Otherwise do the semantic checking to verify that we can wrap the - // branch. - if (!S->mayProduceSingleValue(ctx)) - continue; + BS->setLastElement(SingleValueStmtExpr::createWithWrappedBranches( + ctx, S, DC, mustBeExpr)); } - BS->setLastElement( - SingleValueStmtExpr::createWithWrappedBranches(ctx, S, DC, mustBeExpr)); + + // Wrap single expression branches in an implicit 'then '. + if (auto *E = BS->getSingleActiveExpression()) + BS->setLastElement(ThenStmt::createImplicit(ctx, E)); } return SVE; } @@ -2562,6 +2564,14 @@ SingleValueStmtExpr::tryDigOutSingleValueStmtExpr(Expr *E) { return finder.FoundSVE; } +ThenStmt *SingleValueStmtExpr::getThenStmtFrom(BraceStmt *BS) { + if (BS->empty()) + return nullptr; + + // Must be the last statement in the brace. + return dyn_cast_or_null(BS->getLastElement().dyn_cast()); +} + SourceRange SingleValueStmtExpr::getSourceRange() const { return S->getSourceRange(); } @@ -2588,20 +2598,30 @@ SingleValueStmtExpr::getBranches(SmallVectorImpl &scratch) const { llvm_unreachable("Unhandled case in switch!"); } -ArrayRef SingleValueStmtExpr::getSingleExprBranches( - SmallVectorImpl &scratch) const { +ArrayRef SingleValueStmtExpr::getThenStmts( + SmallVectorImpl &scratch) const { assert(scratch.empty()); SmallVector stmtScratch; for (auto *branch : getBranches(stmtScratch)) { auto *BS = dyn_cast(branch); if (!BS) continue; - if (auto *E = BS->getSingleActiveExpression()) - scratch.push_back(E); + if (auto *TS = getThenStmtFrom(BS)) + scratch.push_back(TS); } return scratch; } +ArrayRef SingleValueStmtExpr::getResultExprs( + SmallVectorImpl &scratch) const { + assert(scratch.empty()); + SmallVector stmtScratch; + for (auto *TS : getThenStmts(stmtScratch)) + scratch.push_back(TS->getResult()); + + return scratch; +} + void InterpolatedStringLiteralExpr::forEachSegment(ASTContext &Ctx, llvm::function_ref callback) { auto appendingExpr = getAppendingExpr(); diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp index b5b35b51de7ae..0966075df8bd1 100644 --- a/lib/AST/Stmt.cpp +++ b/lib/AST/Stmt.cpp @@ -51,6 +51,8 @@ StringRef Stmt::getDescriptiveKindName(StmtKind K) { return "return"; case StmtKind::Yield: return "yield"; + case StmtKind::Then: + return "then"; case StmtKind::Defer: return "defer"; case StmtKind::If: @@ -363,6 +365,26 @@ SourceLoc YieldStmt::getEndLoc() const { return RPLoc.isInvalid() ? getYields()[0]->getEndLoc() : RPLoc; } +ThenStmt *ThenStmt::createParsed(ASTContext &ctx, SourceLoc thenLoc, + Expr *result) { + return new (ctx) ThenStmt(thenLoc, result, /*isImplicit*/ false); +} + +ThenStmt *ThenStmt::createImplicit(ASTContext &ctx, Expr *result) { + return new (ctx) ThenStmt(SourceLoc(), result, /*isImplicit*/ true); +} + +SourceRange ThenStmt::getSourceRange() const { + auto range = getResult()->getSourceRange(); + if (!range) + return ThenLoc; + + if (ThenLoc) + range.widen(ThenLoc); + + return range; +} + SourceLoc ThrowStmt::getEndLoc() const { return SubExpr->getEndLoc(); } SourceLoc DiscardStmt::getEndLoc() const { return SubExpr->getEndLoc(); } diff --git a/lib/ASTGen/Sources/ASTGen/SourceFile.swift b/lib/ASTGen/Sources/ASTGen/SourceFile.swift index 5fc97b01503ca..deeed29911532 100644 --- a/lib/ASTGen/Sources/ASTGen/SourceFile.swift +++ b/lib/ASTGen/Sources/ASTGen/SourceFile.swift @@ -35,6 +35,7 @@ extension Parser.ExperimentalFeatures { insert(feature) } } + mapFeature(.ThenStatements, to: .thenStatements) } } diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index a0aba04d859ea..f6fcf501f3b3b 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -293,6 +293,7 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks, } void completeReturnStmt(CodeCompletionExpr *E) override; + void completeThenStmt(CodeCompletionExpr *E) override; void completeYieldStmt(CodeCompletionExpr *E, llvm::Optional yieldIndex) override; void completeAfterPoundExpr(CodeCompletionExpr *E, @@ -591,6 +592,12 @@ void CodeCompletionCallbacksImpl::completeReturnStmt(CodeCompletionExpr *E) { Kind = CompletionKind::ReturnStmtExpr; } +void CodeCompletionCallbacksImpl::completeThenStmt(CodeCompletionExpr *E) { + CurDeclContext = P.CurDeclContext; + CodeCompleteTokenExpr = E; + Kind = CompletionKind::ThenStmtExpr; +} + void CodeCompletionCallbacksImpl::completeYieldStmt( CodeCompletionExpr *E, llvm::Optional index) { CurDeclContext = P.CurDeclContext; @@ -1052,6 +1059,7 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink, case CompletionKind::ReturnStmtExpr: addKeywordsAfterReturn(Sink, CurDeclContext); LLVM_FALLTHROUGH; + case CompletionKind::ThenStmtExpr: case CompletionKind::YieldStmtExpr: case CompletionKind::ForEachSequence: addSuperKeyword(Sink, CurDeclContext); @@ -1561,11 +1569,19 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) { case CompletionKind::PostfixExprBeginning: case CompletionKind::StmtOrExpr: case CompletionKind::ReturnStmtExpr: - case CompletionKind::YieldStmtExpr: { + case CompletionKind::YieldStmtExpr: + case CompletionKind::ThenStmtExpr: { assert(CurDeclContext); - bool AddUnresolvedMemberCompletions = - (Kind == CompletionKind::CaseStmtBeginning); + bool AddUnresolvedMemberCompletions = false; + switch (Kind) { + case CompletionKind::CaseStmtBeginning: + case CompletionKind::ThenStmtExpr: + AddUnresolvedMemberCompletions = true; + break; + default: + break; + } ExprTypeCheckCompletionCallback Lookup( CodeCompleteTokenExpr, CurDeclContext, AddUnresolvedMemberCompletions); if (CodeCompleteTokenExpr) { @@ -1735,6 +1751,7 @@ void CodeCompletionCallbacksImpl::doneParsing(SourceFile *SrcFile) { case CompletionKind::PostfixExpr: case CompletionKind::ReturnStmtExpr: case CompletionKind::YieldStmtExpr: + case CompletionKind::ThenStmtExpr: llvm_unreachable("should be already handled"); return; diff --git a/lib/IDE/TypeContextInfo.cpp b/lib/IDE/TypeContextInfo.cpp index 1edff74426952..a784ccbf4e155 100644 --- a/lib/IDE/TypeContextInfo.cpp +++ b/lib/IDE/TypeContextInfo.cpp @@ -45,6 +45,7 @@ class ContextInfoCallbacks : public CodeCompletionCallbacks, void completeCallArg(CodeCompletionExpr *E, bool isFirst) override; void completeReturnStmt(CodeCompletionExpr *E) override; + void completeThenStmt(CodeCompletionExpr *E) override; void completeYieldStmt(CodeCompletionExpr *E, llvm::Optional yieldIndex) override; @@ -73,6 +74,10 @@ void ContextInfoCallbacks::completeReturnStmt(CodeCompletionExpr *E) { CurDeclContext = P.CurDeclContext; ParsedExpr = E; } +void ContextInfoCallbacks::completeThenStmt(CodeCompletionExpr *E) { + CurDeclContext = P.CurDeclContext; + ParsedExpr = E; +} void ContextInfoCallbacks::completeYieldStmt( CodeCompletionExpr *E, llvm::Optional yieldIndex) { CurDeclContext = P.CurDeclContext; diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 883c8d5125b91..62985a9c44aa0 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -3578,10 +3578,11 @@ ParserResult Parser::parseExprCollection() { // If The next token is at the beginning of a new line and can never start // an element, break. - if (Tok.isAtStartOfLine() && (Tok.isAny(tok::r_brace, tok::pound_endif) || - isStartOfSwiftDecl() || isStartOfStmt())) + if (Tok.isAtStartOfLine() && + (Tok.isAny(tok::r_brace, tok::pound_endif) || isStartOfSwiftDecl() || + isStartOfStmt(/*preferExpr*/ false))) { break; - + } diagnose(Tok, diag::expected_separator, ",") .fixItInsertAfter(PreviousLoc, ","); Status.setIsParseError(); diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 084cd8e739b9d..2346e4169392d 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -34,7 +34,7 @@ using namespace swift; /// isStartOfStmt - Return true if the current token starts a statement. /// -bool Parser::isStartOfStmt() { +bool Parser::isStartOfStmt(bool preferExpr) { // This needs to be kept in sync with `Parser::parseStmt()`. If a new token // kind is accepted here as start of statement, it should also be handled in // `Parser::parseStmt()`. @@ -84,14 +84,15 @@ bool Parser::isStartOfStmt() { } Parser::BacktrackingScope backtrack(*this); consumeToken(tok::kw_try); - return isStartOfStmt(); + return isStartOfStmt(preferExpr); } case tok::identifier: { // "identifier ':' for/while/do/switch" is a label on a loop/switch. if (!peekToken().is(tok::colon)) { - // "yield" or "discard" in the right context begins a statement. - if (isContextualYieldKeyword() || isContextualDiscardKeyword()) { + // "yield", "discard", and "then" in the right context begins a statement. + if (isContextualYieldKeyword() || isContextualDiscardKeyword() || + isContextualThenKeyword(preferExpr)) { return true; } return false; @@ -111,7 +112,7 @@ bool Parser::isStartOfStmt() { } // For better recovery, we just accept a label on any statement. We reject // putting a label on something inappropriate in parseStmt(). - return isStartOfStmt(); + return isStartOfStmt(preferExpr); } case tok::at_sign: { @@ -123,7 +124,7 @@ bool Parser::isStartOfStmt() { Parser::BacktrackingScope backtrack(*this); consumeToken(tok::at_sign); consumeToken(tok::identifier); - return isStartOfStmt(); + return isStartOfStmt(preferExpr); } } } @@ -146,7 +147,7 @@ ParserStatus Parser::parseExprOrStmt(ASTNode &Result) { return makeParserCodeCompletionStatus(); } - if (isStartOfStmt()) { + if (isStartOfStmt(/*preferExpr*/ false)) { ParserResult Res = parseStmt(); if (Res.isNonNull()) { auto *S = Res.get(); @@ -562,6 +563,9 @@ ParserResult Parser::parseStmt() { Tok.setKind(tok::kw_discard); } + if (isContextualThenKeyword(/*preferExpr*/ false)) + Tok.setKind(tok::kw_then); + // This needs to handle everything that `Parser::isStartOfStmt()` accepts as // start of statement. switch (Tok.getKind()) { @@ -588,6 +592,9 @@ ParserResult Parser::parseStmt() { case tok::kw_yield: if (LabelInfo) diagnose(LabelInfo.Loc, diag::invalid_label_on_stmt); return parseStmtYield(tryLoc); + case tok::kw_then: + if (LabelInfo) diagnose(LabelInfo.Loc, diag::invalid_label_on_stmt); + return parseStmtThen(tryLoc); case tok::kw_throw: if (LabelInfo) diagnose(LabelInfo.Loc, diag::invalid_label_on_stmt); return parseStmtThrow(tryLoc); @@ -697,7 +704,7 @@ static ParserStatus parseOptionalControlTransferTarget(Parser &P, // "break x+y") but since the expression after the them is dead, we don't feel // bad eagerly parsing this. if (!P.Tok.isAtStartOfLine()) { - if (P.Tok.is(tok::identifier) && !P.isStartOfStmt() && + if (P.Tok.is(tok::identifier) && !P.isStartOfStmt(/*preferExpr*/ true) && !P.isStartOfSwiftDecl()) { TargetLoc = P.consumeIdentifier(Target, /*diagnoseDollarPrefix=*/false); return makeParserSuccess(); @@ -775,7 +782,7 @@ ParserResult Parser::parseStmtReturn(SourceLoc tryLoc) { if (Tok.isAny(tok::kw_if, tok::kw_switch)) { return true; } - if (isStartOfStmt() || isStartOfSwiftDecl()) + if (isStartOfStmt(/*preferExpr*/ true) || isStartOfSwiftDecl()) return false; return true; @@ -804,7 +811,7 @@ ParserResult Parser::parseStmtReturn(SourceLoc tryLoc) { } if (tryLoc.isValid()) { - diagnose(tryLoc, diag::try_on_return_throw_yield, /*return=*/0) + diagnose(tryLoc, diag::try_must_come_after_stmt, /*return=*/0) .fixItInsert(ExprLoc, "try ") .fixItRemoveChars(tryLoc, ReturnLoc); @@ -854,7 +861,7 @@ ParserResult Parser::parseStmtYield(SourceLoc tryLoc) { // yielded values, suggest just removing the try instead of // suggesting adding it to every yielded value. if (tryLoc.isValid()) { - diagnose(tryLoc, diag::try_on_return_throw_yield, /*yield=*/2) + diagnose(tryLoc, diag::try_must_come_after_stmt, /*yield=*/2) .fixItRemoveChars(tryLoc, yieldLoc); } @@ -874,7 +881,7 @@ ParserResult Parser::parseStmtYield(SourceLoc tryLoc) { // There's a single yielded value, so suggest moving 'try' before it. if (tryLoc.isValid()) { - diagnose(tryLoc, diag::try_on_return_throw_yield, /*yield=*/2) + diagnose(tryLoc, diag::try_must_come_after_stmt, /*yield=*/2) .fixItInsert(beginLoc, "try ") .fixItRemoveChars(tryLoc, yieldLoc); } @@ -896,6 +903,95 @@ ParserResult Parser::parseStmtYield(SourceLoc tryLoc) { status, YieldStmt::create(Context, yieldLoc, lpLoc, yields, rpLoc)); } +bool Parser::isContextualThenKeyword(bool preferExpr) { + if (!Context.LangOpts.hasFeature(Feature::ThenStatements)) + return false; + + if (!Tok.isContextualKeyword("then")) + return false; + + // If we want to prefer an expr, and aren't at the start of a newline, then + // don't parse a ThenStmt. + if (preferExpr && !Tok.isAtStartOfLine()) + return false; + + // 'then' immediately followed by '('/'[' is a function/subscript call. If + // immediately followed by '.', it's a member access. + if (peekToken().isAny(tok::l_paren, tok::l_square, tok::period)) { + auto tokEndLoc = Lexer::getLocForEndOfToken(SourceMgr, Tok.getLoc()); + return peekToken().getLoc() != tokEndLoc; + } + + // 'then' followed by '{' is a trailing closure on a function call. + if (peekToken().is(tok::l_brace)) + return false; + + // If we have 'then' followed by an infix or postfix operator, we know this + // must be an expression. + if (peekToken().isBinaryOperatorLike() || peekToken().isPostfixOperatorLike()) + return false; + + // These act like binary operators. + if (peekToken().isAny(tok::kw_is, tok::kw_as)) + return false; + + return true; +} + +/// parseStmtThen +/// +/// stmt-then: +/// 'then' expr +/// +ParserResult Parser::parseStmtThen(SourceLoc tryLoc) { + SourceLoc thenLoc = consumeToken(tok::kw_then); + + if (Tok.is(tok::code_complete)) { + auto ccLoc = consumeToken(); + auto *CCE = new (Context) CodeCompletionExpr(ccLoc); + if (CodeCompletionCallbacks) + CodeCompletionCallbacks->completeThenStmt(CCE); + + return makeParserCodeCompletionResult( + ThenStmt::createParsed(Context, thenLoc, CCE)); + } + + auto exprLoc = Tok.getLoc(); + + // Issue a warning when the expression is on a different line than + // the 'then' keyword, but both have the same indentation. + if (SourceMgr.getLineAndColumnInBuffer(thenLoc).second == + SourceMgr.getLineAndColumnInBuffer(exprLoc).second) { + diagnose(exprLoc, diag::unindented_code_after_then); + diagnose(exprLoc, diag::indent_expression_to_silence); + } + + ParserResult result = parseExpr(diag::expected_expr_after_then); + bool hasCodeCompletion = result.hasCodeCompletion(); + + // If we couldn't parse an expr, fill it the gap with an ErrorExpr, as + // ThenStmt expects an expression node. + if (result.isNull()) + result = makeParserErrorResult(new (Context) ErrorExpr(exprLoc)); + + if (tryLoc.isValid()) { + diagnose(tryLoc, diag::try_must_come_after_stmt, /*then=*/3) + .fixItInsert(exprLoc, "try ") + .fixItRemoveChars(tryLoc, thenLoc); + + // Note: We can't use tryLoc here because that's outside the ThenStmt's + // source range. + if (!isa(result.get())) + result = makeParserResult(new (Context) TryExpr(exprLoc, result.get())); + } + + if (hasCodeCompletion) + result.setHasCodeCompletionAndIsError(); + + return makeParserResult( + result, ThenStmt::createParsed(Context, thenLoc, result.get())); +} + /// parseStmtThrow /// /// stmt-throw @@ -914,7 +1010,7 @@ ParserResult Parser::parseStmtThrow(SourceLoc tryLoc) { Result = makeParserErrorResult(new (Context) ErrorExpr(throwLoc)); if (tryLoc.isValid() && exprLoc.isValid()) { - diagnose(tryLoc, diag::try_on_return_throw_yield, /*throw=*/1) + diagnose(tryLoc, diag::try_must_come_after_stmt, /*throw=*/1) .fixItInsert(exprLoc, "try ") .fixItRemoveChars(tryLoc, throwLoc); @@ -2243,7 +2339,8 @@ static bool isStmtForCStyle(Parser &P) { // Skip until we see ';', or something that ends control part. while (true) { if (P.Tok.isAny(tok::eof, tok::kw_in, tok::l_brace, tok::r_brace, - tok::r_paren) || P.isStartOfStmt()) + tok::r_paren) || + P.isStartOfStmt(/*preferExpr*/ false)) return false; // If we saw newline before ';', consider it is a foreach statement. if (!HasLParen && P.Tok.isAtStartOfLine()) diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 2156a51be1dfc..07406b800fe45 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -707,8 +707,10 @@ SourceLoc Parser::skipUntilGreaterInTypeList(bool protocolComposition) { // 'Self' can appear in types, skip it. if (Tok.is(tok::kw_Self)) break; - if (isStartOfStmt() || isStartOfSwiftDecl() || Tok.is(tok::pound_endif)) + if (isStartOfStmt(/*preferExpr*/ false) || isStartOfSwiftDecl() || + Tok.is(tok::pound_endif)) { return lastLoc; + } break; case tok::l_paren: @@ -1019,8 +1021,8 @@ Parser::parseListItem(ParserStatus &Status, tok RightK, SourceLoc LeftLoc, } // If we're in a comma-separated list, the next token is at the // beginning of a new line and can never start an element, break. - if (Tok.isAtStartOfLine() && - (Tok.is(tok::r_brace) || isStartOfSwiftDecl() || isStartOfStmt())) { + if (Tok.isAtStartOfLine() && (Tok.is(tok::r_brace) || isStartOfSwiftDecl() || + isStartOfStmt(/*preferExpr*/ false))) { return ParseListItemResult::Finished; } // If we found EOF or such, bailout. diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index a7ee4ca4706f7..3185f4a30d9d0 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -2235,7 +2235,7 @@ RValue RValueEmitter::visitSingleValueStmtExpr(SingleValueStmtExpr *E, // Collect the target exprs that will be used for initialization. SmallVector scratch; SILGenFunction::SingleValueStmtInitialization initInfo(resultAddr); - for (auto *E : E->getSingleExprBranches(scratch)) + for (auto *E : E->getResultExprs(scratch)) initInfo.Exprs.insert(E); // Push the initialization for branches of the statement to initialize into. diff --git a/lib/SILGen/SILGenStmt.cpp b/lib/SILGen/SILGenStmt.cpp index eb4aaadd9dca5..720e4a4e1a0b1 100644 --- a/lib/SILGen/SILGenStmt.cpp +++ b/lib/SILGen/SILGenStmt.cpp @@ -419,16 +419,7 @@ void StmtEmitter::visitBraceStmt(BraceStmt *S) { if (isa(S)) StmtType = ThrowStmtType; } else if (auto *E = ESD.dyn_cast()) { - // Check to see if we have an initialization for a SingleValueStmtExpr - // active, and if so, use it for this expression branch. If the expression - // is uninhabited, we can skip this, and let unreachability checking - // handle it. - auto init = SGF.getSingleValueStmtInit(E); - if (init && !E->getType()->isStructurallyUninhabited()) { - SGF.emitExprInto(E, init.get()); - } else { - SGF.emitIgnoredExpr(E); - } + SGF.emitIgnoredExpr(E); } else { auto *D = ESD.get(); @@ -834,6 +825,17 @@ void StmtEmitter::visitYieldStmt(YieldStmt *S) { SGF.emitYield(S, sources, origTypes, SGF.CoroutineUnwindDest); } +void StmtEmitter::visitThenStmt(ThenStmt *S) { + auto *E = S->getResult(); + auto init = SGF.getSingleValueStmtInit(E); + + if (init && !E->getType()->isStructurallyUninhabited()) { + SGF.emitExprInto(E, init.get()); + } else { + SGF.emitIgnoredExpr(E); + } +} + void StmtEmitter::visitPoundAssertStmt(PoundAssertStmt *stmt) { SILValue condition; { diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp index 4041e7618233e..1336a4871599e 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp @@ -246,6 +246,7 @@ void DiagnosticEmitter::emitMissingConsumeInDiscardingContext( case StmtKind::Return: case StmtKind::Yield: case StmtKind::Break: + case StmtKind::Then: case StmtKind::Fail: case StmtKind::PoundAssert: return true; diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 3088f5bfc56ad..38fc7f88f0e90 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -163,6 +163,11 @@ class ResultBuilderTransform assert(returnStmt->isImplicit()); element = returnStmt->getResult(); } + // Unwrap an implicit ThenStmt. + if (auto *thenStmt = getAsStmt(element)) { + if (thenStmt->isImplicit()) + element = thenStmt->getResult(); + } if (auto *decl = element.dyn_cast()) { switch (decl->getKind()) { @@ -756,6 +761,7 @@ class ResultBuilderTransform UNSUPPORTED_STMT(Throw) UNSUPPORTED_STMT(Return) UNSUPPORTED_STMT(Yield) + UNSUPPORTED_STMT(Then) UNSUPPORTED_STMT(Discard) UNSUPPORTED_STMT(Defer) UNSUPPORTED_STMT(Guard) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 25d36fcb3436f..0891865a43fcc 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8764,20 +8764,8 @@ namespace { auto resultTy = solution.getResolvedType(SVE); Rewriter.cs.setType(SVE, resultTy); - SmallVector scratch; - SmallPtrSet exprBranches; - for (auto *branch : SVE->getSingleExprBranches(scratch)) - exprBranches.insert(branch); - return Rewriter.cs.applySolutionToSingleValueStmt( solution, SVE, solution.getDC(), [&](SyntacticElementTarget target) { - // We need to fixup the conversion type to the full result type, - // not the branch result type. This is necessary as there may be - // an additional conversion required for the branch. - if (auto *E = target.getAsExpr()) { - if (exprBranches.contains(E)) - target.setExprConversionType(resultTy); - } auto resultTarget = rewriteTarget(target); if (!resultTarget) return resultTarget; diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 9514cf912be09..d343aed6b7fee 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2575,7 +2575,7 @@ bool ContextualFailure::diagnoseAsError() { diagnostic = diag::ternary_expr_cases_mismatch; break; } - case ConstraintLocator::SingleValueStmtBranch: { + case ConstraintLocator::SingleValueStmtResult: { diagnostic = diag::single_value_stmt_branches_mismatch; break; } diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 7d10ce9b761dc..1af8e46eda720 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -3872,10 +3872,10 @@ namespace { if (auto *SVE = expr->getSingleValueStmtExpr()) { // If we have a SingleValueStmtExpr, form a join of the branch types. SmallVector scratch; - auto branches = SVE->getSingleExprBranches(scratch); + auto branches = SVE->getResultExprs(scratch); for (auto idx : indices(branches)) { auto *eltLoc = CS.getConstraintLocator( - SVE, {LocatorPathElt::SingleValueStmtBranch(idx)}); + SVE, {LocatorPathElt::SingleValueStmtResult(idx)}); elements.emplace_back(CS.getType(branches[idx]), eltLoc); } } else { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 7cd9741fa88ec..1888ea73ecfd0 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -6370,7 +6370,7 @@ bool ConstraintSystem::repairFailures( } case ConstraintLocator::TernaryBranch: - case ConstraintLocator::SingleValueStmtBranch: { + case ConstraintLocator::SingleValueStmtResult: { recordAnyTypeVarAsPotentialHole(lhs); recordAnyTypeVarAsPotentialHole(rhs); @@ -7813,16 +7813,20 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, // value statements. // // As with the previous checks, we only allow the Void conversion in - // an implicit single-expression closure. In the more general case, we - // only allow the Never conversion. + // an implicit single-expression closure. In the more general case for + // implicit branches, we only allow the Never conversion. For explicit + // branches, no conversions are allowed. auto *loc = getConstraintLocator(locator); if (auto branchKind = loc->isForSingleValueStmtBranch()) { bool allowConversion = false; switch (*branchKind) { - case SingleValueStmtBranchKind::Regular: + case SingleValueStmtBranchKind::Explicit: + allowConversion = false; + break; + case SingleValueStmtBranchKind::Implicit: allowConversion = type1->isUninhabited(); break; - case SingleValueStmtBranchKind::InSingleExprClosure: + case SingleValueStmtBranchKind::ImplicitInSingleExprClosure: allowConversion = true; break; } @@ -15200,12 +15204,12 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( impact = 5; } } - using SingleValueStmtBranch = LocatorPathElt::SingleValueStmtBranch; - if (auto branchElt = locator->getLastElementAs()) { + using SingleValueStmtResult = LocatorPathElt::SingleValueStmtResult; + if (auto branchElt = locator->getLastElementAs()) { // Similar to a ternary, except we have N branches. Let's prefer the fix // on the first branch, and discount subsequent branches by index. - if (branchElt->getExprBranchIndex() > 0) - impact = 9 + branchElt->getExprBranchIndex(); + if (branchElt->getIndex() > 0) + impact = 9 + branchElt->getIndex(); } // Increase impact of invalid conversions to `Any` and `AnyHashable` // associated with collection elements (i.e. for-in sequence element) diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 60dbc3bc74b0f..461a28820ef9a 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -268,7 +268,7 @@ static bool isViableElement(ASTNode element, // code completion token, as it won't contribute to the type of the // SingleValueStmtExpr. if (isForSingleValueStmtCompletion && - !braceStmt->getSingleActiveExpression() && + !SingleValueStmtExpr::hasResult(braceStmt) && !cs.containsIDEInspectionTarget(braceStmt)) { return false; } @@ -458,6 +458,10 @@ struct SyntacticElementContext return this->dyn_cast(); } + NullablePtr getAsSingleValueStmtExpr() const { + return this->dyn_cast(); + } + llvm::Optional getAsAnyFunctionRef() const { if (auto *fn = this->dyn_cast()) { return {fn}; @@ -1141,14 +1145,13 @@ class SyntacticElementConstraintGenerator for (auto element : braceStmt->getElements()) { if (cs.isForCodeCompletion() && - !cs.containsIDEInspectionTarget(element) && - !(braceStmt->getSingleActiveExpression() && - locator->isForSingleValueStmtConjunctionOrBrace())) { + !cs.containsIDEInspectionTarget(element)) { // To improve performance, skip type checking elements that can't - // influence the code completion token. Note we don't do this for - // single expression SingleValueStmtExpr branches, as they're needed to - // infer the type. - if (element.is() && !element.isStmt(StmtKind::Guard) && !element.isStmt(StmtKind::Return)) { + // influence the code completion token. + if (element.is() && + !element.isStmt(StmtKind::Guard) && + !element.isStmt(StmtKind::Return) && + !element.isStmt(StmtKind::Then)) { // Statements can't influence the expresion that contains the code // completion token. // Guard statements might define variables that are used in the code @@ -1185,17 +1188,6 @@ class SyntacticElementConstraintGenerator isDiscarded = !contextInfo || contextInfo->purpose == CTP_Unused; } - // For an if/switch expression, if the contextual type for the branch is - // still a type variable, we can drop it. This avoids needlessly - // propagating the type of the branch to subsequent branches, instead - // we'll let the join handle the conversion. - if (contextInfo && isExpr(locator->getAnchor())) { - auto contextualFixedTy = cs.getFixedTypeRecursive( - contextInfo->getType(), /*wantRValue*/ true); - if (contextualFixedTy->isTypeVariableOrMember()) - contextInfo = llvm::None; - } - elements.push_back(makeElement( element, cs.getConstraintLocator(locator, @@ -1259,6 +1251,30 @@ class SyntacticElementConstraintGenerator cs.setTargetFor(returnStmt, target); } + void visitThenStmt(ThenStmt *thenStmt) { + auto *resultExpr = thenStmt->getResult(); + auto contextInfo = cs.getContextualTypeInfo(resultExpr); + + // For an if/switch expression, if the contextual type for the branch is + // still a type variable, we can drop it. This avoids needlessly + // propagating the type of the branch to subsequent branches, instead + // we'll let the join handle the conversion. + if (contextInfo && isExpr(locator->getAnchor())) { + auto contextualFixedTy = + cs.getFixedTypeRecursive(contextInfo->getType(), /*wantRValue*/ true); + if (contextualFixedTy->isTypeVariableOrMember()) + contextInfo = llvm::None; + } + + // We form a single element conjunction here to ensure the context type var + // gets taken out of the active type vars (assuming we dropped it) before we + // produce a solution. + auto resultElt = makeElement(resultExpr, locator, + contextInfo.value_or(ContextualTypeInfo()), + /*isDiscarded=*/false); + createConjunction(cs, {resultElt}, locator); + } + ContextualTypeInfo getContextualResultInfo() const { auto funcRef = AnyFunctionRef::fromDeclContext(context.getAsDeclContext()); if (!funcRef) @@ -1408,15 +1424,15 @@ bool ConstraintSystem::generateConstraints(SingleValueStmtExpr *E) { Type resultTy = createTypeVariable(loc, /*options*/ 0); setType(E, resultTy); - // Assign contextual types for each of the expression branches. + // Assign contextual types for each of the result exprs. SmallVector scratch; - auto branches = E->getSingleExprBranches(scratch); + auto branches = E->getResultExprs(scratch); for (auto idx : indices(branches)) { auto *branch = branches[idx]; auto ctpElt = LocatorPathElt::ContextualType(CTP_SingleValueStmtBranch); auto *loc = getConstraintLocator( - E, {LocatorPathElt::SingleValueStmtBranch(idx), ctpElt}); + E, {LocatorPathElt::SingleValueStmtResult(idx), ctpElt}); ContextualTypeInfo info(resultTy, CTP_SingleValueStmtBranch, loc); setContextualInfo(branch, info); @@ -2159,6 +2175,38 @@ class SyntacticElementSolutionApplication return returnStmt; } + ASTNode visitThenStmt(ThenStmt *thenStmt) { + // Note we're defensive here over whether we're in a SingleValueStmtExpr, + // as we don't diagnose out-of-place ThenStmts until MiscDiagnostics. If we + // have an out-of-place ThenStmt, just rewrite the expr without a contextual + // type. + auto SVE = context.getAsSingleValueStmtExpr(); + auto ty = SVE ? solution.getResolvedType(SVE.get()) : Type(); + + // We need to fixup the conversion type to the full result type, + // not the branch result type. This is necessary as there may be + // an additional conversion required for the branch. + auto target = solution.getTargetFor(thenStmt->getResult()); + if (ty) + target->setExprConversionType(ty); + + auto *resultExpr = thenStmt->getResult(); + if (auto newResultTarget = rewriteTarget(*target)) + resultExpr = newResultTarget->getAsExpr(); + + thenStmt->setResult(resultExpr); + + if (!SVE) + return thenStmt; + + // If the expression was typed as Void, its branches are effectively + // discarded, so treat them as ignored expressions. + if (ty->lookThroughAllOptionalTypes()->isVoid()) { + TypeChecker::checkIgnoredExpr(resultExpr); + } + return thenStmt; + } + #define UNSUPPORTED_STMT(STMT) ASTNode visit##STMT##Stmt(STMT##Stmt *) { \ llvm_unreachable("Unsupported statement kind " #STMT); \ } @@ -2592,16 +2640,6 @@ bool ConstraintSystem::applySolutionToSingleValueStmt( if (!stmt || application.hadError) return true; - // If the expression was typed as Void, its branches are effectively - // discarded, so treat them as ignored expressions. This doesn't happen in - // the solution application walker as we consider all the branches to have - // contextual types. - if (solution.getResolvedType(SVE)->lookThroughAllOptionalTypes()->isVoid()) { - SmallVector scratch; - for (auto *branch : SVE->getSingleExprBranches(scratch)) - TypeChecker::checkIgnoredExpr(branch); - } - SVE->setStmt(stmt); return false; } diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index cfaa47acdb783..8a8d3f95f2371 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -103,7 +103,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const { case ConstraintLocator::PackExpansionPattern: case ConstraintLocator::PatternBindingElement: case ConstraintLocator::NamedPatternDecl: - case ConstraintLocator::SingleValueStmtBranch: + case ConstraintLocator::SingleValueStmtResult: case ConstraintLocator::AnyPatternDecl: case ConstraintLocator::GlobalActorType: case ConstraintLocator::CoercionOperand: @@ -393,9 +393,9 @@ void LocatorPathElt::dump(raw_ostream &out) const { break; } - case ConstraintLocator::SingleValueStmtBranch: { - auto branch = elt.castTo(); - out << "expr branch [" << branch.getExprBranchIndex() << "] of " + case ConstraintLocator::SingleValueStmtResult: { + auto branch = elt.castTo(); + out << "result [" << branch.getIndex() << "] of " "single value stmt"; break; } @@ -688,24 +688,31 @@ llvm::Optional ConstraintLocator::isForSingleValueStmtBranch() const { // Ignore a trailing ContextualType path element. auto path = getPath(); - if (auto elt = getLastElementAs()) + if (isLastElement()) path = path.drop_back(); if (path.empty()) return llvm::None; - if (!path.back().is()) + auto resultElt = path.back().getAs(); + if (!resultElt) return llvm::None; auto *SVE = getAsExpr(getAnchor()); if (!SVE) return llvm::None; + // Check to see if we have an explicit result, i.e 'then '. + SmallVector scratch; + auto *TS = SVE->getThenStmts(scratch)[resultElt->getIndex()]; + if (!TS->isImplicit()) + return SingleValueStmtBranchKind::Explicit; + if (auto *CE = dyn_cast(SVE->getDeclContext())) { if (CE->hasSingleExpressionBody() && !hasExplicitResult(CE)) - return SingleValueStmtBranchKind::InSingleExprClosure; + return SingleValueStmtBranchKind::ImplicitInSingleExprClosure; } - return SingleValueStmtBranchKind::Regular; + return SingleValueStmtBranchKind::Implicit; } NullablePtr ConstraintLocator::getPatternMatch() const { diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e3969fc48a178..d487e05762753 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -5859,12 +5859,12 @@ void constraints::simplifyLocator(ASTNode &anchor, continue; } - case ConstraintLocator::SingleValueStmtBranch: { - auto branchElt = path[0].castTo(); - auto exprIdx = branchElt.getExprBranchIndex(); + case ConstraintLocator::SingleValueStmtResult: { + auto branchElt = path[0].castTo(); + auto exprIdx = branchElt.getIndex(); auto *SVE = castToExpr(anchor); SmallVector scratch; - anchor = SVE->getSingleExprBranches(scratch)[exprIdx]; + anchor = SVE->getResultExprs(scratch)[exprIdx]; path = path.slice(1); continue; } diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 88836e3116c3d..7508ab824bc17 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -3846,6 +3846,7 @@ class SingleValueStmtUsageChecker final : public ASTWalker { ASTContext &Ctx; DiagnosticEngine &Diags; llvm::DenseSet ValidSingleValueStmtExprs; + llvm::DenseSet ValidThenStmts; public: SingleValueStmtUsageChecker( @@ -3918,10 +3919,10 @@ class SingleValueStmtUsageChecker final : public ASTWalker { SVE->getStmt()->getKind()); } - // Nested SingleValueStmtExprs are allowed. - SmallVector scratch; - for (auto *branch : SVE->getSingleExprBranches(scratch)) - markValidSingleValueStmt(branch); + // Mark the valid 'then' statements. + SmallVector scratch; + for (auto *thenStmt : SVE->getThenStmts(scratch)) + ValidThenStmts.insert(thenStmt); // Diagnose invalid SingleValueStmtExprs. This should only happen for // expressions in positions that we didn't support before @@ -3942,7 +3943,7 @@ class SingleValueStmtUsageChecker final : public ASTWalker { } } Diags.diagnose(branch->getEndLoc(), - diag::single_value_stmt_branch_must_end_in_throw, + diag::single_value_stmt_branch_must_end_in_result, S->getKind()); } break; @@ -3971,7 +3972,7 @@ class SingleValueStmtUsageChecker final : public ASTWalker { } break; } - case IsSingleValueStmtResult::Kind::NoExpressionBranches: + case IsSingleValueStmtResult::Kind::NoResult: // This is fine, we will have typed the expression as Void (we verify // as such in the ASTVerifier). break; @@ -3999,7 +4000,7 @@ class SingleValueStmtUsageChecker final : public ASTWalker { } PreWalkResult walkToStmtPre(Stmt *S) override { - // Valid in a return/throw. + // Valid in a return/throw/then. if (auto *RS = dyn_cast(S)) { if (RS->hasResult()) markValidSingleValueStmt(RS->getResult()); @@ -4007,6 +4008,12 @@ class SingleValueStmtUsageChecker final : public ASTWalker { if (auto *TS = dyn_cast(S)) markValidSingleValueStmt(TS->getSubExpr()); + if (auto *TS = dyn_cast(S)) { + markValidSingleValueStmt(TS->getResult()); + if (!ValidThenStmts.contains(TS)) + Diags.diagnose(TS->getThenLoc(), diag::out_of_place_then_stmt); + } + return Action::Continue(S); } diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 13bbbb2fbd158..abde198ae90d8 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -1179,7 +1179,18 @@ class StmtChecker : public StmtVisitor { } return YS; } - + + Stmt *visitThenStmt(ThenStmt *TS) { + // These are invalid outside of if/switch expressions, but let's type-check + // to get extra diagnostics. + getASTContext().Diags.diagnose(TS->getThenLoc(), + diag::out_of_place_then_stmt); + auto *E = TS->getResult(); + TypeChecker::typeCheckExpression(E, DC); + TS->setResult(E); + return TS; + } + Stmt *visitThrowStmt(ThrowStmt *TS) { // Coerce the operand to the exception type. auto E = TS->getSubExpr(); @@ -2519,18 +2530,17 @@ bool TypeCheckASTNodeAtLocRequest::evaluate( if (isa(E)) return Action::SkipChildren(E); - // If the location is within a single-expression branch of a - // SingleValueStmtExpr, walk it directly rather than as part of the brace. - // This ensures we type-check it a part of the whole expression, unless it - // has an inner closure or SVE, in which case we can still pick up a - // better node to type-check. + // If the location is within a result of a SingleValueStmtExpr, walk it + // directly rather than as part of the brace. This ensures we type-check + // it a part of the whole expression, unless it has an inner closure or + // SVE, in which case we can still pick up a better node to type-check. if (auto *SVE = dyn_cast(E)) { SmallVector scratch; - for (auto *branch : SVE->getSingleExprBranches(scratch)) { - auto branchCharRange = Lexer::getCharSourceRangeFromSourceRange( - SM, branch->getSourceRange()); - if (branchCharRange.contains(Loc)) { - if (!branch->walk(*this)) + for (auto *result : SVE->getResultExprs(scratch)) { + auto resultCharRange = Lexer::getCharSourceRangeFromSourceRange( + SM, result->getSourceRange()); + if (resultCharRange.contains(Loc)) { + if (!result->walk(*this)) return Action::Stop(); return Action::SkipChildren(E); @@ -2857,20 +2867,6 @@ void TypeChecker::typeCheckTopLevelCodeDecl(TopLevelCodeDecl *TLCD) { performTopLevelDeclDiagnostics(TLCD); } -/// Whether the given brace statement ends with a throw. -static bool doesBraceEndWithThrow(BraceStmt *BS) { - auto elts = BS->getElements(); - if (elts.empty()) - return false; - - auto lastElt = elts.back(); - auto *S = lastElt.dyn_cast(); - if (!S) - return false; - - return isa(S); -} - namespace { /// An ASTWalker that searches for any break/continue/return statements that /// jump out of the context the walker starts at. @@ -2924,6 +2920,38 @@ class JumpOutOfContextFinder : public ASTWalker { }; } // end anonymous namespace +/// Whether the given brace statement ends with a 'throw'. +static bool doesBraceEndWithThrow(BraceStmt *BS) { + if (BS->empty()) + return false; + + auto *S = BS->getLastElement().dyn_cast(); + if (!S) + return false; + + return isa(S); +} + +/// Whether the given brace statement is considered to produce a result for +/// an if/switch expression. +static bool doesBraceProduceResult(BraceStmt *BS, Evaluator &eval) { + if (BS->empty()) + return false; + + // We consider the branch as having a result if there is: + // - A single active expression or statement that can be turned into an + // expression. + // - 'then ' as the last statement. + if (BS->getSingleActiveExpression()) + return true; + + if (auto *S = BS->getSingleActiveStatement()) { + if (S->mayProduceSingleValue(eval)) + return true; + } + return SingleValueStmtExpr::hasResult(BS); +} + IsSingleValueStmtResult areBranchesValidForSingleValueStmt(Evaluator &eval, ArrayRef branches) { TinyPtrVector invalidJumps; @@ -2932,7 +2960,7 @@ areBranchesValidForSingleValueStmt(Evaluator &eval, ArrayRef branches) { // Must have a single expression brace, and non-single-expression branches // must end with a throw. - bool hadSingleExpr = false; + bool hadResult = false; for (auto *branch : branches) { auto *BS = dyn_cast(branch); if (!BS) @@ -2941,19 +2969,13 @@ areBranchesValidForSingleValueStmt(Evaluator &eval, ArrayRef branches) { // Check to see if there are any invalid jumps. BS->walk(jumpFinder); - if (BS->getSingleActiveExpression()) { - hadSingleExpr = true; + // Check to see if a result is produced from the branch. + if (doesBraceProduceResult(BS, eval)) { + hadResult = true; continue; } - // We also allow single value statement branches, which we can wrap in - // a SingleValueStmtExpr. - if (auto *S = BS->getSingleActiveStatement()) { - if (S->mayProduceSingleValue(eval)) { - hadSingleExpr = true; - continue; - } - } + // If there was no result, the branch must end in a 'throw'. if (!doesBraceEndWithThrow(BS)) unterminatedBranches.push_back(BS); } @@ -2966,8 +2988,9 @@ areBranchesValidForSingleValueStmt(Evaluator &eval, ArrayRef branches) { std::move(unterminatedBranches)); } - if (!hadSingleExpr) - return IsSingleValueStmtResult::noExpressionBranches(); + // No branches produced a result, we can't turn this into an expression. + if (!hadResult) + return IsSingleValueStmtResult::noResult(); return IsSingleValueStmtResult::valid(); } diff --git a/test/Constraints/then_stmt.swift b/test/Constraints/then_stmt.swift new file mode 100644 index 0000000000000..ed0eb0dea863a --- /dev/null +++ b/test/Constraints/then_stmt.swift @@ -0,0 +1,109 @@ +// RUN: %target-typecheck-verify-swift -enable-experimental-feature ThenStatements + +// Required for experimental features +// REQUIRES: asserts + +enum E { + case e + case f + case g(Int) +} + +func testVoidNeverConversion() { + // Explicit 'then' statements don't participate in implicit Void/Never conversions. + let _: () -> Void = { + if .random() { + then 0 // expected-error {{cannot convert value of type 'Int' to specified type 'Void'}} + } else { + 0 + } + } + let _: () -> Void = { + if .random() { + then fatalError() // expected-error {{cannot convert value of type 'Never' to specified type 'Void'}} + } else { + 0 + } + } + let _ = { + if .random() { + then "" // expected-error {{branches have mismatching types 'String' and 'Int'}} + } else { + then 0 + } + } + let _ = { + if .random() { + then () + } else { + 0 // expected-warning {{integer literal is unused}} + } + } + let _ = { + if .random() { + then "" + } else { + fatalError() + } + } + func foo() -> Int { + switch Bool.random() { + case true: + fatalError() + case false: + then fatalError() // expected-error {{cannot convert value of type 'Never' to specified type 'Int'}} + } + } +} + +enum Either { + case first(T), second(U) +} + +@resultBuilder +struct Builder { // expected-note 2{{struct 'Builder' declared here}} + static func buildBlock(_ x: T) -> T { x } + static func buildBlock(_ x: T, _ y: U) -> (T, U) { (x, y) } + + static func buildEither(first x: T) -> Either { .first(x) } + static func buildEither(second x: U) -> Either { .second(x) } + + static func buildExpression(_ x: Double) -> Double { x } + static func buildExpression(_ x: T) -> T { x } +} + +@Builder +func testThenStmtBuilder1() -> Either { + // We don't allow explicit 'then' to be part of the transform. + if .random() { + then 0 // expected-error {{closure containing control flow statement cannot be used with result builder 'Builder'}} + } else { + 1 + } +} + +@Builder +func testThenStmtBuilder2() -> Either { + // We don't allow explicit 'then' to be part of the transform. + if .random() { + then 0 // expected-error {{closure containing control flow statement cannot be used with result builder 'Builder'}} + } else { + 1 + } + () +} + +@Builder +func testThenStmtBuilder3() -> Either { + // But it's fine as part of e.g a binding. + let x = if .random() { + then 0 + } else { + 1 + } + if .random() { + x + } else { + "" + } +} diff --git a/test/IDE/complete_then_stmt.swift b/test/IDE/complete_then_stmt.swift new file mode 100644 index 0000000000000..c53eff2b5ec59 --- /dev/null +++ b/test/IDE/complete_then_stmt.swift @@ -0,0 +1,84 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -enable-experimental-feature ThenStatements -debug-forbid-typecheck-prefix FORBIDDEN -filecheck %raw-FileCheck -completion-output-dir %t + +// Experimental feature requires asserts +// REQUIRES: asserts + +enum E { + case e + case f(Int) +} + +struct NO { + // Triggering interface type computation of this will cause an assert to fire, + // ensuring we don't attempt to type-check any references to it. + static var TYPECHECK = FORBIDDEN +} + +func testThenStmt1() -> E { + if .random() { + () + then .#^DOT1?check=DOT^# + } else { + .e + } +} + +func testThenStmt2() -> E { + switch Bool.random() { + case true: + .e + case false: + () + then .#^DOT2?check=DOT^# + } +} + +func testThenStmt3() throws -> E { + switch Bool.random() { + case true: + NO.TYPECHECK + throw NO.TYPECHECK + case false: + () + then .#^DOT3?check=DOT^# + } +} + +func testThenStmt4() throws -> E { + switch Bool.random() { + case true: + NO.TYPECHECK + throw NO.TYPECHECK + case false: + then #^NODOT1?check=NODOT^# + } +} + +// NODOT: Begin completions +// NODOT-DAG: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: .e[#E#]; name=e +// NODOT-DAG: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: .f({#Int#})[#E#]; name=f() + +struct S { + var e: E +} + +func testThenStmt5() throws -> E { + let x = switch Bool.random() { + case true: + NO.TYPECHECK + throw NO.TYPECHECK + case false: + let s = S(e: .e) + then s.#^SDOT1?check=SDOT^# + } + return x +} + +// SDOT: Begin completions, 2 items +// SDOT-DAG: Keyword[self]/CurrNominal: self[#S#]; name=self +// SDOT-DAG: Decl[InstanceVar]/CurrNominal: e[#E#]; name=e + +// DOT: Begin completions, 2 items +// DOT-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: e[#E#]; name=e +// DOT-DAG: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: f({#Int#})[#E#]; name=f() diff --git a/test/stmt/print/then_stmt.swift b/test/stmt/print/then_stmt.swift new file mode 100644 index 0000000000000..9d9676818d5b0 --- /dev/null +++ b/test/stmt/print/then_stmt.swift @@ -0,0 +1,18 @@ +// RUN: %target-swift-frontend -print-ast -enable-experimental-feature ThenStatements %s 2>&1 | %FileCheck %s + +// Required for experimental features +// REQUIRES: asserts + +func foo() -> Int { + if .random() { + then 0 + } else { + 0 + } + // Make sure we don't print implicit 'then' statements. + // CHECK: return if Bool.random() { + // CHECK-NEXT: {{^}} then 0 + // CHECK-NEXT: } else { + // CHECK-NEXT: {{^}} 0 + // CHECK-NEXT: } +} diff --git a/test/stmt/then_stmt.swift b/test/stmt/then_stmt.swift new file mode 100644 index 0000000000000..ab81db4116b93 --- /dev/null +++ b/test/stmt/then_stmt.swift @@ -0,0 +1,220 @@ +// RUN: %target-typecheck-verify-swift -enable-bare-slash-regex -disable-availability-checking -enable-experimental-feature ThenStatements + +// Required for experimental features +// REQUIRES: asserts + +// Required for regex +// REQUIRES: swift_in_compiler + +func then(_: Int = 0, x: Int = 0, fn: () -> Void = {}) {} + +func testThenStmt(_ x: Int) { + // These are statements + then x // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then () // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then (1) // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then (1, 2) // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then "" // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then [] + // expected-error@-1 {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + // expected-error@-2 {{empty collection literal requires an explicit type}} + then [0] // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + + then if .random() { 0 } else { 1 } + // expected-error@-1 {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + // expected-error@-2 {{'if' may only be used as expression in return, throw, or as the source of an assignment}} + + then -1 // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then ~1 // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then /abc/ // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + + let x: Int = if .random() { + then .zero + } else { + then 0 + } + let y = if .random() { then x } else { then 1 } + let _ = if .random() { (); then y } else { then 1 } + + // We don't allow labels on 'then' statements. + let _ = switch Bool.random() { + case true: + a: then 0 // expected-error {{labels are only valid on loops, if, and switch statements}} + case false: + then 1 + } +} + +func testThenFunctionCalls() { + // These should all be treated as function calls. + then() + then(0) + then(x: 0) + then{} + then {} +} + +func testThenLabel() { + then: for then in [0] { // expected-warning {{immutable value 'then' was never used; consider replacing with '_' or removing it}} + break then + continue then + } +} + +struct S { + var then: Int + func then(_ x: Int) {} + + subscript(x: Int) -> Void { () } + + mutating func testThenAsMember() -> Int { + do { + then // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + } // expected-error {{expected expression after 'then'}} + then // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + 0 // expected-warning {{expression following 'then' is treated as an argument of the 'then'}} + // expected-note@-1 {{indent the expression to silence this warning}} + + // Indented is okay. + then // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + 0 + + then; + // expected-error@-1 {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + // expected-error@-2 {{expected expression after 'then'}} + + then . foo + // expected-error@-1 {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + // expected-error@-2 {{reference to member 'foo' cannot be resolved without a contextual type}} + + // These are expressions. + let _ = then + let _ = self.then + then + 1 // expected-warning {{result of operator '+' is unused}} + then = 2 + + (then) // expected-warning {{property is accessed but result is unused}} + + then is Int + // expected-warning@-1 {{'is' test is always true}} + // expected-warning@-2 {{expression of type 'Bool' is unused}} + then as Int + // expected-warning@-1 {{expression of type 'Int' is unused}} + then as? Int + // expected-warning@-1 {{conditional cast from 'Int' to 'Int' always succeeds}} + // expected-warning@-2 {{expression of type 'Int?' is unused}} + then as! Int + // expected-warning@-1 {{forced cast of 'Int' to same type has no effect}} + // expected-warning@-2 {{expression of type 'Int' is unused}} + + then ? 0 : 1 // expected-error {{type 'Int' cannot be used as a boolean; test for '!= 0' instead}} + then! // expected-error {{cannot force unwrap value of non-optional type 'Int'}} + then? // expected-error {{cannot use optional chaining on non-optional value of type 'Int'}} + then.bitWidth // expected-warning {{expression of type 'Int' is unused}} + then?.bitWidth // expected-error {{cannot use optional chaining on non-optional value of type 'Int'}} + then!.bitWidth // expected-error {{cannot force unwrap value of non-optional type 'Int'}} + + // This is tricky because the lexer considers '/^' to be an infix operator. + then /^ then/ + // expected-error@-1 {{cannot find operator '/^' in scope}} + // expected-error@-2 {{'/' is not a postfix unary operator}} + + self.then(0) + return then + } + func testThenSubscript() { + let then = self + then[0] + } +} + +func testOutOfPlace() -> Int { + if .random() { + guard .random() else { + then 0 // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + } + if .random() { + then 0 // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + } else { + then 1 // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + } + then 0 // expected-error {{'then' may only appear as the last statement in an 'if' or 'switch' expression}} + then 0 + } else { + then 1 + } +} + +func testNested1() -> Int { + if .random() { + if .random() { + then 1 + } else { + then 2 + } + } else { + switch Bool.random() { + case true: + then 0 + case false: + then 1 + } + } +} + +func testNested2() -> Int { + if .random() { + then if .random() { + then 1 + } else { + then 2 + } + } else { + then switch Bool.random() { + case true: + then 0 + case false: + then 1 + } + } +} + +func testNested3() -> Int { + if .random() { + print("hello") + then if .random() { + then 1 + } else { + then 2 + } + } else { + () + () + then switch Bool.random() { + case true: + then 0 + case false: + then 1 + } + } +} + +func throwingFn() throws -> Int { 0 } + +func testTryOnThen() throws -> Int { + switch 0 { + case 1: + then try throwingFn() // okay + default: + try then() // expected-warning {{no calls to throwing functions occur within 'try' expression}} + try then{} // expected-warning {{no calls to throwing functions occur within 'try' expression}} + try then {} // expected-warning {{no calls to throwing functions occur within 'try' expression}} + + try then throwingFn() + // expected-error@-1 {{'try' must be placed on the produced expression}} {{5-9=}} {{14-14=try }} + } +} + +func testReturnTryThen(_ then: Int) -> Int { + return try then // expected-warning {{no calls to throwing functions occur within 'try' expression}} +} diff --git a/test/stmt/then_stmt_disabled.swift b/test/stmt/then_stmt_disabled.swift new file mode 100644 index 0000000000000..803174d30ad32 --- /dev/null +++ b/test/stmt/then_stmt_disabled.swift @@ -0,0 +1,11 @@ +// RUN: %target-typecheck-verify-swift + +// Then statements are disabled by default +let x = if .random() { + print("hello") + then 0 + // expected-error@-1 {{cannot find 'then' in scope}} + // expected-error@-2 {{consecutive statements on a line must be separated by ';'}} +} else { + 1 +} diff --git a/test/stmt/then_stmt_exec.swift b/test/stmt/then_stmt_exec.swift new file mode 100644 index 0000000000000..015af68ade378 --- /dev/null +++ b/test/stmt/then_stmt_exec.swift @@ -0,0 +1,32 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swiftc_driver -Xfrontend -enable-experimental-feature -Xfrontend ThenStatements -o %t/a.out %s +// RUN: %target-codesign %t/a.out +// RUN: %target-run %t/a.out | %FileCheck %s + +// REQUIRES: executable_test + +// Required for experimental features +// REQUIRES: asserts + +func testDefer(_ x: Bool) -> Int { + defer { + print("defer fn") + } + let x = if x { + defer { + print("defer if") + } + print("enter if") + then 0 + } else { + 1 + } + print("after if") + return x +} +_ = testDefer(true) + +// CHECK: enter if +// CHECK-NEXT: defer if +// CHECK-NEXT: after if +// CHECK-NEXT: defer fn From c0ae9b950b4d566ea0b7f08840584a1f370af312 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Fri, 1 Sep 2023 14:32:15 +0100 Subject: [PATCH 2/2] Move out-of-place ThenStmt checking to constraint generation --- include/swift/Sema/CSFix.h | 26 ++++++++++++++++++++++++++ lib/Sema/CSDiagnostics.cpp | 5 +++++ lib/Sema/CSDiagnostics.h | 10 ++++++++++ lib/Sema/CSFix.cpp | 12 ++++++++++++ lib/Sema/CSSimplify.cpp | 1 + lib/Sema/CSSyntacticElement.cpp | 25 ++++++++++++++----------- lib/Sema/MiscDiagnostics.cpp | 11 +---------- 7 files changed, 69 insertions(+), 21 deletions(-) diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h index b3b458ea2f3f6..0bdd3235e7d19 100644 --- a/include/swift/Sema/CSFix.h +++ b/include/swift/Sema/CSFix.h @@ -458,6 +458,9 @@ enum class FixKind : uint8_t { /// Ignore an attempt to specialize non-generic type. AllowConcreteTypeSpecialization, + /// Ignore an out-of-place \c then statement. + IgnoreOutOfPlaceThenStmt, + /// Ignore situations when provided number of generic arguments didn't match /// expected number of parameters. IgnoreGenericSpecializationArityMismatch, @@ -3684,6 +3687,29 @@ class AllowConcreteTypeSpecialization final : public ConstraintFix { } }; +class IgnoreOutOfPlaceThenStmt final : public ConstraintFix { + IgnoreOutOfPlaceThenStmt(ConstraintSystem &cs, ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::IgnoreOutOfPlaceThenStmt, locator) {} + +public: + std::string getName() const override { + return "ignore out-of-place ThenStmt"; + } + + bool diagnose(const Solution &solution, bool asNote = false) const override; + + bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override { + return diagnose(*commonFixes.front().first); + } + + static IgnoreOutOfPlaceThenStmt *create(ConstraintSystem &cs, + ConstraintLocator *locator); + + static bool classof(const ConstraintFix *fix) { + return fix->getKind() == FixKind::IgnoreOutOfPlaceThenStmt; + } +}; + class IgnoreGenericSpecializationArityMismatch final : public ConstraintFix { ValueDecl *D; unsigned NumParams; diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index d343aed6b7fee..843dc1e522639 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -9158,6 +9158,11 @@ bool ConcreteTypeSpecialization::diagnoseAsError() { return true; } +bool OutOfPlaceThenStmtFailure::diagnoseAsError() { + emitDiagnostic(diag::out_of_place_then_stmt); + return true; +} + bool InvalidTypeSpecializationArity::diagnoseAsError() { emitDiagnostic(diag::type_parameter_count_mismatch, D->getBaseIdentifier(), NumParams, NumArgs, NumArgs < NumParams, HasParameterPack); diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index 6bf182bce03ab..1467e4cb8a5d9 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -3073,6 +3073,16 @@ class ConcreteTypeSpecialization final : public FailureDiagnostic { bool diagnoseAsError() override; }; +/// Diagnose an out-of-place ThenStmt. +class OutOfPlaceThenStmtFailure final : public FailureDiagnostic { +public: + OutOfPlaceThenStmtFailure(const Solution &solution, + ConstraintLocator *locator) + : FailureDiagnostic(solution, locator) {} + + bool diagnoseAsError() override; +}; + /// Diagnose attempts to specialize with invalid number of generic arguments: /// /// \code diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 7926851c38235..8b8dbf87d2e93 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -2843,6 +2843,18 @@ AllowConcreteTypeSpecialization::create(ConstraintSystem &cs, Type concreteTy, AllowConcreteTypeSpecialization(cs, concreteTy, locator); } +bool IgnoreOutOfPlaceThenStmt::diagnose(const Solution &solution, + bool asNote) const { + OutOfPlaceThenStmtFailure failure(solution, getLocator()); + return failure.diagnose(asNote); +} + +IgnoreOutOfPlaceThenStmt * +IgnoreOutOfPlaceThenStmt::create(ConstraintSystem &cs, + ConstraintLocator *locator) { + return new (cs.getAllocator()) IgnoreOutOfPlaceThenStmt(cs, locator); +} + bool IgnoreGenericSpecializationArityMismatch::diagnose( const Solution &solution, bool asNote) const { InvalidTypeSpecializationArity failure(solution, D, NumParams, NumArgs, diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 1888ea73ecfd0..3e7da8595614a 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -15291,6 +15291,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::DestructureTupleToMatchPackExpansionParameter: case FixKind::AllowValueExpansionWithoutPackReferences: case FixKind::IgnoreInvalidPatternInExpr: + case FixKind::IgnoreOutOfPlaceThenStmt: case FixKind::IgnoreMissingEachKeyword: llvm_unreachable("handled elsewhere"); } diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 461a28820ef9a..ebd616057b343 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -1255,11 +1255,21 @@ class SyntacticElementConstraintGenerator auto *resultExpr = thenStmt->getResult(); auto contextInfo = cs.getContextualTypeInfo(resultExpr); + // First check to make sure the ThenStmt is in a valid position. + SmallVector validThenStmts; + if (auto SVE = context.getAsSingleValueStmtExpr()) + (void)SVE.get()->getThenStmts(validThenStmts); + + if (!llvm::is_contained(validThenStmts, thenStmt)) { + auto *thenLoc = cs.getConstraintLocator(thenStmt); + (void)cs.recordFix(IgnoreOutOfPlaceThenStmt::create(cs, thenLoc)); + } + // For an if/switch expression, if the contextual type for the branch is // still a type variable, we can drop it. This avoids needlessly // propagating the type of the branch to subsequent branches, instead // we'll let the join handle the conversion. - if (contextInfo && isExpr(locator->getAnchor())) { + if (contextInfo) { auto contextualFixedTy = cs.getFixedTypeRecursive(contextInfo->getType(), /*wantRValue*/ true); if (contextualFixedTy->isTypeVariableOrMember()) @@ -2176,19 +2186,15 @@ class SyntacticElementSolutionApplication } ASTNode visitThenStmt(ThenStmt *thenStmt) { - // Note we're defensive here over whether we're in a SingleValueStmtExpr, - // as we don't diagnose out-of-place ThenStmts until MiscDiagnostics. If we - // have an out-of-place ThenStmt, just rewrite the expr without a contextual - // type. auto SVE = context.getAsSingleValueStmtExpr(); - auto ty = SVE ? solution.getResolvedType(SVE.get()) : Type(); + assert(SVE && "Should have diagnosed an out-of-place ThenStmt"); + auto ty = solution.getResolvedType(SVE.get()); // We need to fixup the conversion type to the full result type, // not the branch result type. This is necessary as there may be // an additional conversion required for the branch. auto target = solution.getTargetFor(thenStmt->getResult()); - if (ty) - target->setExprConversionType(ty); + target->setExprConversionType(ty); auto *resultExpr = thenStmt->getResult(); if (auto newResultTarget = rewriteTarget(*target)) @@ -2196,9 +2202,6 @@ class SyntacticElementSolutionApplication thenStmt->setResult(resultExpr); - if (!SVE) - return thenStmt; - // If the expression was typed as Void, its branches are effectively // discarded, so treat them as ignored expressions. if (ty->lookThroughAllOptionalTypes()->isVoid()) { diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 7508ab824bc17..bbfd97356ec7e 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -3846,7 +3846,6 @@ class SingleValueStmtUsageChecker final : public ASTWalker { ASTContext &Ctx; DiagnosticEngine &Diags; llvm::DenseSet ValidSingleValueStmtExprs; - llvm::DenseSet ValidThenStmts; public: SingleValueStmtUsageChecker( @@ -3919,11 +3918,6 @@ class SingleValueStmtUsageChecker final : public ASTWalker { SVE->getStmt()->getKind()); } - // Mark the valid 'then' statements. - SmallVector scratch; - for (auto *thenStmt : SVE->getThenStmts(scratch)) - ValidThenStmts.insert(thenStmt); - // Diagnose invalid SingleValueStmtExprs. This should only happen for // expressions in positions that we didn't support before // (e.g assignment or *explicit* return). @@ -4008,11 +4002,8 @@ class SingleValueStmtUsageChecker final : public ASTWalker { if (auto *TS = dyn_cast(S)) markValidSingleValueStmt(TS->getSubExpr()); - if (auto *TS = dyn_cast(S)) { + if (auto *TS = dyn_cast(S)) markValidSingleValueStmt(TS->getResult()); - if (!ValidThenStmts.contains(TS)) - Diags.diagnose(TS->getThenLoc(), diag::out_of_place_then_stmt); - } return Action::Continue(S); }