[ConstraintSystem] Warn about discarded expressions found in multi-statement closures

xedin · xedin · commit 46ff410a23f6 · 2021-12-03T10:53:50.000-08:00
diff --git a/include/swift/Sema/Constraint.h b/include/swift/Sema/Constraint.h
@@ -442,6 +442,8 @@ class Constraint final : public llvm::ilist_node<Constraint>,
       ASTNode Element;
       /// Contextual information associated with the element (if any).
       ContextualTypeInfo Context;
+      /// Identifies whether result of this node is unused.
+      bool IsDiscarded;
     } ClosureElement;
   };
 
@@ -495,7 +497,7 @@ class Constraint final : public llvm::ilist_node<Constraint>,
              SmallPtrSetImpl<TypeVariableType *> &typeVars);
 
   /// Construct a closure body element constraint.
-  Constraint(ASTNode node, ContextualTypeInfo context,
+  Constraint(ASTNode node, ContextualTypeInfo context, bool isDiscarded,
              ConstraintLocator *locator,
              SmallPtrSetImpl<TypeVariableType *> &typeVars);
 
@@ -585,12 +587,14 @@ class Constraint final : public llvm::ilist_node<Constraint>,
 
   static Constraint *createClosureBodyElement(ConstraintSystem &cs,
                                               ASTNode node,
-                                              ConstraintLocator *locator);
+                                              ConstraintLocator *locator,
+                                              bool isDiscarded = false);
 
   static Constraint *createClosureBodyElement(ConstraintSystem &cs,
                                               ASTNode node,
                                               ContextualTypeInfo context,
-                                              ConstraintLocator *locator);
+                                              ConstraintLocator *locator,
+                                              bool isDiscarded = false);
 
   /// Determine the kind of constraint.
   ConstraintKind getKind() const { return Kind; }
@@ -857,6 +861,11 @@ class Constraint final : public llvm::ilist_node<Constraint>,
     return ClosureElement.Context;
   }
 
+  bool isDiscardedElement() const {
+    assert(Kind == ConstraintKind::ClosureBodyElement);
+    return ClosureElement.IsDiscarded;
+  }
+
   /// For an applicable function constraint, retrieve the trailing closure
   /// matching rule.
   Optional<TrailingClosureMatching> getTrailingClosureMatching() const;
diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h
@@ -4854,8 +4854,8 @@ class ConstraintSystem {
   /// Simplify a closure body element constraint by generating required
   /// constraints to represent the given element in constraint system.
   SolutionKind simplifyClosureBodyElementConstraint(
-      ASTNode element, ContextualTypeInfo context, TypeMatchOptions flags,
-      ConstraintLocatorBuilder locator);
+      ASTNode element, ContextualTypeInfo context, bool isDiscarded,
+      TypeMatchOptions flags, ConstraintLocatorBuilder locator);
 
 public: // FIXME: Public for use by static functions.
   /// Simplify a conversion constraint with a fix applied to it.
diff --git a/lib/Sema/CSClosure.cpp b/lib/Sema/CSClosure.cpp
@@ -162,8 +162,8 @@ static bool isViableElement(ASTNode element) {
   return true;
 }
 
-using ElementInfo =
-    std::tuple<ASTNode, ContextualTypeInfo, ConstraintLocator *>;
+using ElementInfo = std::tuple<ASTNode, ContextualTypeInfo,
+                               /*isDiscarded=*/bool, ConstraintLocator *>;
 
 static void createConjunction(ConstraintSystem &cs,
                               ArrayRef<ElementInfo> elements,
@@ -190,7 +190,8 @@ static void createConjunction(ConstraintSystem &cs,
   for (const auto &entry : elements) {
     ASTNode element = std::get<0>(entry);
     ContextualTypeInfo context = std::get<1>(entry);
-    ConstraintLocator *elementLoc = std::get<2>(entry);
+    bool isDiscarded = std::get<2>(entry);
+    ConstraintLocator *elementLoc = std::get<3>(entry);
 
     if (!isViableElement(element))
       continue;
@@ -201,8 +202,8 @@ static void createConjunction(ConstraintSystem &cs,
     if (isIsolated)
       element.walk(paramCollector);
 
-    constraints.push_back(
-        Constraint::createClosureBodyElement(cs, element, context, elementLoc));
+    constraints.push_back(Constraint::createClosureBodyElement(
+        cs, element, context, elementLoc, isDiscarded));
   }
 
   // It's possible that there are no viable elements in the body,
@@ -220,8 +221,9 @@ static void createConjunction(ConstraintSystem &cs,
 }
 
 ElementInfo makeElement(ASTNode node, ConstraintLocator *locator,
-                        ContextualTypeInfo context = ContextualTypeInfo()) {
-  return std::make_tuple(node, context, locator);
+                        ContextualTypeInfo context = ContextualTypeInfo(),
+                        bool isDiscarded = false) {
+  return std::make_tuple(node, context, isDiscarded, locator);
 }
 
 static ProtocolDecl *getSequenceProtocol(ASTContext &ctx, SourceLoc loc,
@@ -751,6 +753,8 @@ class ClosureConstraintGenerator
 
   void visitBraceStmt(BraceStmt *braceStmt) {
     if (isSupportedMultiStatementClosure()) {
+      auto &ctx = cs.getASTContext();
+
       if (isChildOf(StmtKind::Case)) {
         auto *caseStmt = cast<CaseStmt>(
             locator->castLastElementTo<LocatorPathElt::ClosureBodyElement>()
@@ -765,10 +769,15 @@ class ClosureConstraintGenerator
 
       SmallVector<ElementInfo, 4> elements;
       for (auto element : braceStmt->getElements()) {
+        bool isDiscarded =
+            element.is<Expr *>() &&
+            (!ctx.LangOpts.Playground && !ctx.LangOpts.DebuggerSupport);
+
         elements.push_back(makeElement(
             element,
             cs.getConstraintLocator(
-                locator, LocatorPathElt::ClosureBodyElement(element))));
+                locator, LocatorPathElt::ClosureBodyElement(element)),
+            /*contextualInfo=*/{}, isDiscarded));
       }
 
       createConjunction(cs, elements, locator);
@@ -925,28 +934,22 @@ bool isConditionOfStmt(ConstraintLocatorBuilder locator) {
 
 ConstraintSystem::SolutionKind
 ConstraintSystem::simplifyClosureBodyElementConstraint(
-    ASTNode element, ContextualTypeInfo context, TypeMatchOptions flags,
-    ConstraintLocatorBuilder locator) {
+    ASTNode element, ContextualTypeInfo context, bool isDiscarded,
+    TypeMatchOptions flags, ConstraintLocatorBuilder locator) {
   auto *closure = castToExpr<ClosureExpr>(locator.getAnchor());
 
   ClosureConstraintGenerator generator(*this, closure,
                                        getConstraintLocator(locator));
 
   if (auto *expr = element.dyn_cast<Expr *>()) {
-    if (context.purpose != CTP_Unused) {
-      SolutionApplicationTarget target(expr, closure, context.purpose,
-                                       context.getType(),
-                                       /*isDiscarded=*/false);
-
-      if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
-        return SolutionKind::Error;
-
-      setSolutionApplicationTarget(expr, target);
-      return SolutionKind::Solved;
-    }
+    SolutionApplicationTarget target(expr, closure, context.purpose,
+                                     context.getType(), isDiscarded);
 
-    if (!generateConstraints(expr, closure, /*isInputExpression=*/false))
+    if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
       return SolutionKind::Error;
+
+    setSolutionApplicationTarget(expr, target);
+    return SolutionKind::Solved;
   } else if (auto *stmt = element.dyn_cast<Stmt *>()) {
     generator.visit(stmt);
   } else if (auto *cond = element.dyn_cast<StmtCondition *>()) {
@@ -1241,13 +1244,20 @@ class ClosureConstraintApplication
   }
 
   ASTNode visitBraceStmt(BraceStmt *braceStmt) {
+    auto &cs = solution.getConstraintSystem();
+
     for (auto &node : braceStmt->getElements()) {
       if (auto expr = node.dyn_cast<Expr *>()) {
         // Rewrite the expression.
-        if (auto rewrittenExpr = rewriteExpr(expr))
-          node = rewrittenExpr;
-        else
+        auto target = *cs.getSolutionApplicationTarget(expr);
+        if (auto rewrittenTarget = rewriteTarget(target)) {
+          node = rewrittenTarget->getAsExpr();
+
+          if (target.isDiscardedExpr())
+            TypeChecker::checkIgnoredExpr(castToExpr(node));
+        } else {
           hadError = true;
+        }
       } else if (auto stmt = node.dyn_cast<Stmt *>()) {
         node = visit(stmt);
       } else {
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -12709,6 +12709,7 @@ ConstraintSystem::simplifyConstraint(const Constraint &constraint) {
   case ConstraintKind::ClosureBodyElement:
     return simplifyClosureBodyElementConstraint(
         constraint.getClosureElement(), constraint.getElementContext(),
+        constraint.isDiscardedElement(),
         /*flags=*/None, constraint.getLocator());
 
   case ConstraintKind::BindTupleOfFunctionParams:
diff --git a/lib/Sema/Constraint.cpp b/lib/Sema/Constraint.cpp
@@ -258,13 +258,14 @@ Constraint::Constraint(ConstraintKind kind, ConstraintFix *fix, Type first,
 }
 
 Constraint::Constraint(ASTNode node, ContextualTypeInfo context,
-                       ConstraintLocator *locator,
+                       bool isDiscarded, ConstraintLocator *locator,
                        SmallPtrSetImpl<TypeVariableType *> &typeVars)
     : Kind(ConstraintKind::ClosureBodyElement), TheFix(nullptr),
       HasRestriction(false), IsActive(false), IsDisabled(false),
       IsDisabledForPerformance(false), RememberChoice(false), IsFavored(false),
       IsIsolated(false),
-      NumTypeVariables(typeVars.size()), ClosureElement{node, context},
+      NumTypeVariables(typeVars.size()), ClosureElement{node, context,
+                                                        isDiscarded},
       Locator(locator) {
   std::copy(typeVars.begin(), typeVars.end(), getTypeVariablesBuffer().begin());
 }
@@ -343,7 +344,8 @@ Constraint *Constraint::clone(ConstraintSystem &cs) const {
                   getLocator());
 
   case ConstraintKind::ClosureBodyElement:
-    return createClosureBodyElement(cs, getClosureElement(), getLocator());
+    return createClosureBodyElement(cs, getClosureElement(), getLocator(),
+                                    isDiscardedElement());
   }
 
   llvm_unreachable("Unhandled ConstraintKind in switch.");
@@ -1012,18 +1014,21 @@ Constraint *Constraint::createApplicableFunction(
 
 Constraint *Constraint::createClosureBodyElement(ConstraintSystem &cs,
                                                  ASTNode node,
-                                                 ConstraintLocator *locator) {
-  return createClosureBodyElement(cs, node, ContextualTypeInfo(), locator);
+                                                 ConstraintLocator *locator,
+                                                 bool isDiscarded) {
+  return createClosureBodyElement(cs, node, ContextualTypeInfo(), locator,
+                                  isDiscarded);
 }
 
 Constraint *Constraint::createClosureBodyElement(ConstraintSystem &cs,
                                                  ASTNode node,
                                                  ContextualTypeInfo context,
-                                                 ConstraintLocator *locator) {
+                                                 ConstraintLocator *locator,
+                                                 bool isDiscarded) {
   SmallPtrSet<TypeVariableType *, 4> typeVars;
   unsigned size = totalSizeToAlloc<TypeVariableType *>(typeVars.size());
   void *mem = cs.getAllocator().Allocate(size, alignof(Constraint));
-  return new (mem) Constraint(node, context, locator, typeVars);
+  return new (mem) Constraint(node, context, isDiscarded, locator, typeVars);
 }
 
 Optional<TrailingClosureMatching>
