Merge pull request #68344 from hamishknight/do-it

hamishknight · web-flow · commit 042ccdda017a · 2023-10-10T10:26:12.000+01:00
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -1259,6 +1259,9 @@ ERROR(single_value_stmt_must_be_unlabeled,none,
 ERROR(if_expr_must_be_syntactically_exhaustive,none,
       "'if' must have an unconditional 'else' to be used as expression",
       ())
+ERROR(do_catch_expr_must_be_syntactically_exhaustive,none,
+      "'do catch' must have an unconditional 'catch' to be used as expression",
+      ())
 ERROR(single_value_stmt_branch_empty,none,
       "expected expression in branch of '%0' expression",
       (StmtKind))
@@ -1272,8 +1275,8 @@ ERROR(cannot_jump_in_single_value_stmt,none,
 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", ())
+      "'then' may only appear as the last statement in an 'if', 'switch', or "
+      "'do' 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
@@ -6075,7 +6075,7 @@ class KeyPathDotExpr : public Expr {
 class SingleValueStmtExpr : public Expr {
 public:
   enum class Kind {
-    If, Switch
+    If, Switch, Do, DoCatch
   };
 
 private:
diff --git a/include/swift/AST/Stmt.h b/include/swift/AST/Stmt.h
@@ -144,7 +144,6 @@ class alignas(8) Stmt : public ASTAllocated<Stmt> {
 
   /// Whether the statement can produce a single value, and as such may be
   /// treated as an expression.
-  IsSingleValueStmtResult mayProduceSingleValue(Evaluator &eval) const;
   IsSingleValueStmtResult mayProduceSingleValue(ASTContext &ctx) const;
 
   /// isImplicit - Determines whether this statement was implicitly-generated,
@@ -1418,6 +1417,9 @@ class DoCatchStmt final
     return {getTrailingObjects<CaseStmt *>(), Bits.DoCatchStmt.NumCatches};
   }
 
+  /// Retrieve the complete set of branches for this do-catch statement.
+  ArrayRef<Stmt *> getBranches(SmallVectorImpl<Stmt *> &scratch) const;
+
   /// Does this statement contain a syntactically exhaustive catch
   /// clause?
   ///
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -3947,6 +3947,10 @@ class IsSingleValueStmtResult {
     /// The statement is an 'if' statement without an unconditional 'else'.
     NonExhaustiveIf,
 
+    /// The statement is a 'do catch' statement without an unconditional
+    /// 'catch'.
+    NonExhaustiveDoCatch,
+
     /// There is no branch that produces a resulting value.
     NoResult,
 
@@ -4003,6 +4007,9 @@ class IsSingleValueStmtResult {
   static IsSingleValueStmtResult nonExhaustiveIf() {
     return IsSingleValueStmtResult(Kind::NonExhaustiveIf);
   }
+  static IsSingleValueStmtResult nonExhaustiveDoCatch() {
+    return IsSingleValueStmtResult(Kind::NonExhaustiveDoCatch);
+  }
   static IsSingleValueStmtResult noResult() {
     return IsSingleValueStmtResult(Kind::NoResult);
   }
@@ -4039,18 +4046,21 @@ class IsSingleValueStmtResult {
 };
 
 /// Computes whether a given statement can be treated as a SingleValueStmtExpr.
+///
+// TODO: We ought to consider storing a reference to the ASTContext on the
+// evaluator.
 class IsSingleValueStmtRequest
     : public SimpleRequest<IsSingleValueStmtRequest,
-                           IsSingleValueStmtResult(const Stmt *),
+                           IsSingleValueStmtResult(const Stmt *, ASTContext *),
                            RequestFlags::Cached> {
 public:
   using SimpleRequest::SimpleRequest;
 
 private:
   friend SimpleRequest;
 
-  IsSingleValueStmtResult
-  evaluate(Evaluator &evaluator, const Stmt *stmt) const;
+  IsSingleValueStmtResult evaluate(Evaluator &evaluator, const Stmt *stmt,
+                                   ASTContext *ctx) const;
 
 public:
   bool isCached() const { return true; }
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -448,7 +448,7 @@ SWIFT_REQUEST(TypeChecker, PreCheckReturnStmtRequest,
               Stmt *(ReturnStmt *, DeclContext *),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, IsSingleValueStmtRequest,
-              IsSingleValueStmtResult(const Stmt *),
+              IsSingleValueStmtResult(const Stmt *, ASTContext *),
               Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, MacroDefinitionRequest,
               MacroDefinition(MacroDecl *),
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
@@ -232,6 +232,9 @@ EXPERIMENTAL_FEATURE(PlaygroundExtendedCallbacks, true)
 /// Enable 'then' statements.
 EXPERIMENTAL_FEATURE(ThenStatements, false)
 
+/// Enable 'do' expressions.
+EXPERIMENTAL_FEATURE(DoExpressions, false)
+
 /// Enable the `@_rawLayout` attribute.
 EXPERIMENTAL_FEATURE(RawLayout, true)
 
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
@@ -3563,6 +3563,10 @@ static bool usesFeatureThenStatements(Decl *decl) {
   return false;
 }
 
+static bool usesFeatureDoExpressions(Decl *decl) {
+  return false;
+}
+
 static bool usesFeatureNewCxxMethodSafetyHeuristics(Decl *decl) {
   return decl->hasClangNode();
 }
diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp
@@ -1227,6 +1227,7 @@ class Verifier : public ASTWalker {
         break;
       case Kind::UnterminatedBranches:
       case Kind::NonExhaustiveIf:
+      case Kind::NonExhaustiveDoCatch:
       case Kind::UnhandledStmt:
       case Kind::CircularReference:
       case Kind::HasLabel:
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
@@ -2477,6 +2477,8 @@ SingleValueStmtExpr *SingleValueStmtExpr::create(ASTContext &ctx, Stmt *S,
 
 SingleValueStmtExpr *SingleValueStmtExpr::createWithWrappedBranches(
     ASTContext &ctx, Stmt *S, DeclContext *DC, bool mustBeExpr) {
+  assert(!(isa<DoStmt>(S) || isa<DoCatchStmt>(S)) ||
+         ctx.LangOpts.hasFeature(Feature::DoExpressions));
   auto *SVE = create(ctx, S, DC);
 
   // Attempt to wrap any branches that can be wrapped.
@@ -2488,12 +2490,15 @@ SingleValueStmtExpr *SingleValueStmtExpr::createWithWrappedBranches(
 
     if (auto *S = BS->getSingleActiveStatement()) {
       if (mustBeExpr) {
-        // If this must be an expression, we can eagerly wrap any exhaustive if
-        // and switch branch.
+        // If this must be an expression, we can eagerly wrap any exhaustive if,
+        // switch, and do statement.
         if (auto *IS = dyn_cast<IfStmt>(S)) {
           if (!IS->isSyntacticallyExhaustive())
             continue;
-        } else if (!isa<SwitchStmt>(S)) {
+        } else if (auto *DCS = dyn_cast<DoCatchStmt>(S)) {
+          if (!DCS->isSyntacticallyExhaustive())
+            continue;
+        } else if (!isa<SwitchStmt>(S) && !isa<DoStmt>(S)) {
           continue;
         }
       } else {
@@ -2576,18 +2581,28 @@ SingleValueStmtExpr::Kind SingleValueStmtExpr::getStmtKind() const {
     return Kind::If;
   case StmtKind::Switch:
     return Kind::Switch;
+  case StmtKind::Do:
+    return Kind::Do;
+  case StmtKind::DoCatch:
+    return Kind::DoCatch;
   default:
     llvm_unreachable("Unhandled kind!");
   }
 }
 
 ArrayRef<Stmt *>
 SingleValueStmtExpr::getBranches(SmallVectorImpl<Stmt *> &scratch) const {
+  assert(scratch.empty());
   switch (getStmtKind()) {
   case Kind::If:
     return cast<IfStmt>(getStmt())->getBranches(scratch);
   case Kind::Switch:
     return cast<SwitchStmt>(getStmt())->getBranches(scratch);
+  case Kind::Do:
+    scratch.push_back(cast<DoStmt>(getStmt())->getBody());
+    return scratch;
+  case Kind::DoCatch:
+    return cast<DoCatchStmt>(getStmt())->getBranches(scratch);
   }
   llvm_unreachable("Unhandled case in switch!");
 }
diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp
@@ -333,13 +333,9 @@ Stmt *BraceStmt::getSingleActiveStatement() const {
   return getSingleActiveElement().dyn_cast<Stmt *>();
 }
 
-IsSingleValueStmtResult Stmt::mayProduceSingleValue(Evaluator &eval) const {
-  return evaluateOrDefault(eval, IsSingleValueStmtRequest{this},
-                           IsSingleValueStmtResult::circularReference());
-}
-
 IsSingleValueStmtResult Stmt::mayProduceSingleValue(ASTContext &ctx) const {
-  return mayProduceSingleValue(ctx.evaluator);
+  return evaluateOrDefault(ctx.evaluator, IsSingleValueStmtRequest{this, &ctx},
+                           IsSingleValueStmtResult::circularReference());
 }
 
 SourceLoc ReturnStmt::getStartLoc() const {
@@ -863,6 +859,15 @@ SwitchStmt::getBranches(SmallVectorImpl<Stmt *> &scratch) const {
   return scratch;
 }
 
+ArrayRef<Stmt *>
+DoCatchStmt::getBranches(SmallVectorImpl<Stmt *> &scratch) const {
+  assert(scratch.empty());
+  scratch.push_back(getBody());
+  for (auto *CS : getCatches())
+    scratch.push_back(CS->getBody());
+  return scratch;
+}
+
 // See swift/Basic/Statistic.h for declaration: this enables tracing Stmts, is
 // defined here to avoid too much layering violation / circular linkage
 // dependency.
diff --git a/lib/ASTGen/Sources/ASTGen/SourceFile.swift b/lib/ASTGen/Sources/ASTGen/SourceFile.swift
@@ -37,6 +37,7 @@ extension Parser.ExperimentalFeatures {
     }
     mapFeature(.ThenStatements, to: .thenStatements)
     mapFeature(.TypedThrows, to: .typedThrows)
+    mapFeature(.DoExpressions, to: .doExpressions)
   }
 }
 
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
@@ -592,11 +592,13 @@ ParserResult<Expr> Parser::parseExprUnary(Diag<> Message, bool isExprBasic) {
   // First check to see if we have the start of a regex literal `/.../`.
   tryLexRegexLiteral(/*forUnappliedOperator*/ false);
 
-  // Try parse an 'if' or 'switch' as an expression. Note we do this here in
-  // parseExprUnary as we don't allow postfix syntax to hang off such
+  // Try parse 'if', 'switch', and 'do' as expressions. Note we do this here
+  // in parseExprUnary as we don't allow postfix syntax to hang off such
   // expressions to avoid ambiguities such as postfix '.member', which can
   // currently be parsed as a static dot member for a result builder.
-  if (Tok.isAny(tok::kw_if, tok::kw_switch)) {
+  if (Tok.isAny(tok::kw_if, tok::kw_switch) ||
+      (Tok.is(tok::kw_do) &&
+       Context.LangOpts.hasFeature(Feature::DoExpressions))) {
     auto Result = parseStmt();
     Expr *E = nullptr;
     if (Result.isNonNull()) {
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
@@ -78,8 +78,10 @@ bool Parser::isStartOfStmt(bool preferExpr) {
     // "try" cannot actually start any statements, but we parse it there for
     // better recovery in cases like 'try return'.
 
-    // For 'if' and 'switch' we can parse as an expression.
-    if (peekToken().isAny(tok::kw_if, tok::kw_switch)) {
+    // For 'if', 'switch', and 'do' we can parse as an expression.
+    if (peekToken().isAny(tok::kw_if, tok::kw_switch) ||
+        (peekToken().is(tok::kw_do) &&
+         Context.LangOpts.hasFeature(Feature::DoExpressions))) {
       return false;
     }
     Parser::BacktrackingScope backtrack(*this);
@@ -157,7 +159,10 @@ ParserStatus Parser::parseExprOrStmt(ASTNode &Result) {
       // We could also achieve this by more eagerly attempting to parse an 'if'
       // or 'switch' as an expression when in statement position, but that
       // could result in less useful recovery behavior.
-      if ((isa<IfStmt>(S) || isa<SwitchStmt>(S)) && Tok.is(tok::kw_as)) {
+      if ((isa<IfStmt>(S) || isa<SwitchStmt>(S) ||
+           ((isa<DoStmt>(S) || isa<DoCatchStmt>(S)) &&
+            Context.LangOpts.hasFeature(Feature::DoExpressions))) &&
+          Tok.is(tok::kw_as)) {
         auto *SVE = SingleValueStmtExpr::createWithWrappedBranches(
             Context, S, CurDeclContext, /*mustBeExpr*/ true);
         auto As = parseExprAs();
@@ -778,8 +783,10 @@ ParserResult<Stmt> Parser::parseStmtReturn(SourceLoc tryLoc) {
                   tok::pound_else, tok::pound_elseif)) {
       return false;
     }
-    // Allowed for if/switch expressions.
-    if (Tok.isAny(tok::kw_if, tok::kw_switch)) {
+    // Allowed for if/switch/do expressions.
+    if (Tok.isAny(tok::kw_if, tok::kw_switch) ||
+        (Tok.is(tok::kw_do) &&
+         Context.LangOpts.hasFeature(Feature::DoExpressions))) {
       return true;
     }
     if (isStartOfStmt(/*preferExpr*/ true) || isStartOfSwiftDecl())
diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp
@@ -507,6 +507,9 @@ class SyntacticElementConstraintGenerator
   SyntacticElementContext context;
   ConstraintLocator *locator;
 
+  /// Whether a conjunction was generated.
+  bool generatedConjunction = false;
+
 public:
   /// Whether an error was encountered while generating constraints.
   bool hadError = false;
@@ -519,6 +522,16 @@ class SyntacticElementConstraintGenerator
   void createConjunction(ArrayRef<ElementInfo> elements,
                          ConstraintLocator *locator, bool isIsolated = false,
                          ArrayRef<TypeVariableType *> extraTypeVars = {}) {
+    assert(!generatedConjunction && "Already generated conjunction");
+    generatedConjunction = true;
+
+    // Inject a join if we have one.
+    SmallVector<ElementInfo, 4> scratch;
+    if (auto *join = context.ElementJoin.getPtrOrNull()) {
+      scratch.append(elements.begin(), elements.end());
+      scratch.push_back(makeJoinElement(cs, join, locator));
+      elements = scratch;
+    }
     ::createConjunction(cs, context.getAsDeclContext(), elements, locator,
                         isIsolated, extraTypeVars);
   }
@@ -865,10 +878,6 @@ class SyntacticElementConstraintGenerator
       elements.push_back(makeElement(ifStmt->getElseStmt(), elseLoc));
     }
 
-    // Inject a join if we have one.
-    if (auto *join = context.ElementJoin.getPtrOrNull())
-      elements.push_back(makeJoinElement(cs, join, locator));
-
     createConjunction(elements, locator);
   }
 
@@ -1010,10 +1019,6 @@ class SyntacticElementConstraintGenerator
         elements.push_back(makeElement(rawCase, switchLoc));
     }
 
-    // Inject a join if we have one.
-    if (auto *join = context.ElementJoin.getPtrOrNull())
-      elements.push_back(makeJoinElement(cs, join, switchLoc));
-
     createConjunction(elements, switchLoc);
   }
 
@@ -1023,8 +1028,13 @@ class SyntacticElementConstraintGenerator
 
     SmallVector<ElementInfo, 4> elements;
 
-    // First, let's record a body of `do` statement.
-    elements.push_back(makeElement(doStmt->getBody(), doLoc));
+    // First, let's record a body of `do` statement. Note we need to add a
+    // SyntaticElement locator path element here to avoid treating the inner
+    // brace conjunction as being isolated if 'doLoc' is for an isolated
+    // conjunction (as is the case with 'do' expressions).
+    auto *doBodyLoc = cs.getConstraintLocator(
+        doLoc, LocatorPathElt::SyntacticElement(doStmt->getBody()));
+    elements.push_back(makeElement(doStmt->getBody(), doBodyLoc));
 
     // After that has been type-checked, let's switch to
     // individual `catch` statements.
diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp
@@ -671,7 +671,8 @@ bool ConstraintLocator::isForSingleValueStmtConjunction() const {
 }
 
 bool ConstraintLocator::isForSingleValueStmtConjunctionOrBrace() const {
-  if (!isExpr<SingleValueStmtExpr>(getAnchor()))
+  auto *SVE = getAsExpr<SingleValueStmtExpr>(getAnchor());
+  if (!SVE)
     return false;
 
   auto path = getPath();
@@ -682,11 +683,17 @@ bool ConstraintLocator::isForSingleValueStmtConjunctionOrBrace() const {
       continue;
     }
 
-    // Ignore a SyntaticElement path element for a case statement of a switch.
+    // Ignore a SyntaticElement path element for a case statement of a switch,
+    // or the catch of a do-catch, or the brace of a do-statement.
     if (auto elt = path.back().getAs<LocatorPathElt::SyntacticElement>()) {
       if (elt->getElement().isStmt(StmtKind::Case)) {
         path = path.drop_back();
-        continue;
+        break;
+      }
+      if (elt->getElement().isStmt(StmtKind::Brace) &&
+          isa<DoCatchStmt>(SVE->getStmt())) {
+        path = path.drop_back();
+        break;
       }
     }
     break;
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -3947,6 +3947,11 @@ class SingleValueStmtUsageChecker final : public ASTWalker {
                        diag::if_expr_must_be_syntactically_exhaustive);
         break;
       }
+      case IsSingleValueStmtResult::Kind::NonExhaustiveDoCatch: {
+        Diags.diagnose(S->getStartLoc(),
+                       diag::do_catch_expr_must_be_syntactically_exhaustive);
+        break;
+      }
       case IsSingleValueStmtResult::Kind::HasLabel: {
         // FIXME: We should offer a fix-it to remove (currently we don't track
         // the colon SourceLoc).
diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp
diff --git a/test/SILGen/do_expr.swift b/test/SILGen/do_expr.swift
diff --git a/test/SILGen/do_expr_address_only_tuple_return_value_context.swift b/test/SILGen/do_expr_address_only_tuple_return_value_context.swift
diff --git a/test/expr/unary/do_expr.swift b/test/expr/unary/do_expr.swift
diff --git a/test/expr/unary/do_expr_disabled.swift b/test/expr/unary/do_expr_disabled.swift
diff --git a/test/expr/unary/if_expr.swift b/test/expr/unary/if_expr.swift
diff --git a/test/stmt/then_stmt.swift b/test/stmt/then_stmt.swift

Original file line number	Diff line number	Diff line change
`@@ -3563,6 +3563,10 @@ static bool usesFeatureThenStatements(Decl *decl) {`
`3563`	`3563`	`return false;`
`3564`	`3564`	`}`
`3565`	`3565`
	`3566`	`+static bool usesFeatureDoExpressions(Decl *decl) {`
	`3567`	`+ return false;`
	`3568`	`+}`
	`3569`	`+`
`3566`	`3570`	`static bool usesFeatureNewCxxMethodSafetyHeuristics(Decl *decl) {`
`3567`	`3571`	`return decl->hasClangNode();`
`3568`	`3572`	`}`