Merge pull request swiftlang#33762 from xedin/rdar-65983237

[ConstraintSystem] Extend invalid function body fix to cover constraint generation failures

Author: xedin

include/swift/AST/DiagnosticEngine.h

@@ -1058,10 +1058,19 @@ namespace swift {
       }
     }
 
-    bool hasDiagnostics() const {
-      return std::distance(Engine.TentativeDiagnostics.begin() +
-                               PrevDiagnostics,
-                           Engine.TentativeDiagnostics.end()) > 0;
+    bool hasErrors() const {
+      ArrayRef<Diagnostic> diagnostics(Engine.TentativeDiagnostics.begin() +
+                                           PrevDiagnostics,
+                                       Engine.TentativeDiagnostics.end());
+
+      for (auto &diagnostic : diagnostics) {
+        auto behavior = Engine.state.determineBehavior(diagnostic.getID());
+        if (behavior == DiagnosticState::Behavior::Fatal ||
+            behavior == DiagnosticState::Behavior::Error)
+          return true;
+      }
+
+      return false;
     }
 
     /// Abort and close this transaction and erase all diagnostics

lib/Sema/BuilderTransform.cpp

@@ -1655,6 +1655,13 @@ ConstraintSystem::matchFunctionBuilder(
   assert(builder && "Bad function builder type");
   assert(builder->getAttrs().hasAttribute<FunctionBuilderAttr>());
 
+  if (InvalidFunctionBuilderBodies.count(fn)) {
+    (void)recordFix(
+        IgnoreInvalidFunctionBuilderBody::duringConstraintGeneration(
+            *this, getConstraintLocator(fn.getBody())));
+    return getTypeMatchSuccess();
+  }
+
   // Pre-check the body: pre-check any expressions in it and look
   // for return statements.
   auto request =
@@ -1669,7 +1676,7 @@ ConstraintSystem::matchFunctionBuilder(
     if (!shouldAttemptFixes())
       return getTypeMatchFailure(locator);
 
-    if (recordFix(IgnoreInvalidFunctionBuilderBody::create(
+    if (recordFix(IgnoreInvalidFunctionBuilderBody::duringPreCheck(
             *this, getConstraintLocator(fn.getBody()))))
       return getTypeMatchFailure(locator);
 
@@ -1711,9 +1718,25 @@ ConstraintSystem::matchFunctionBuilder(
   BuilderClosureVisitor visitor(getASTContext(), this, dc, builderType,
                                 bodyResultType);
 
-  auto applied = visitor.apply(fn.getBody());
-  if (!applied)
-    return getTypeMatchFailure(locator);
+  Optional<AppliedBuilderTransform> applied = None;
+  {
+    DiagnosticTransaction transaction(dc->getASTContext().Diags);
+
+    applied = visitor.apply(fn.getBody());
+    if (!applied)
+      return getTypeMatchFailure(locator);
+
+    if (transaction.hasErrors()) {
+      InvalidFunctionBuilderBodies.insert(fn);
+
+      if (recordFix(
+              IgnoreInvalidFunctionBuilderBody::duringConstraintGeneration(
+                  *this, getConstraintLocator(fn.getBody()))))
+        return getTypeMatchFailure(locator);
+
+      return getTypeMatchSuccess();
+    }
+  }
 
   Type transformedType = getType(applied->returnExpr);
   assert(transformedType && "Missing type");
@@ -1800,7 +1823,7 @@ class PreCheckFunctionBuilderApplication : public ASTWalker {
 
       HasError |= ConstraintSystem::preCheckExpression(
           E, DC, /*replaceInvalidRefsWithErrors=*/false);
-      HasError |= transaction.hasDiagnostics();
+      HasError |= transaction.hasErrors();
 
       if (SuppressDiagnostics)
         transaction.abort();

lib/Sema/CSFix.cpp

@@ -1554,6 +1554,14 @@ AllowKeyPathWithoutComponents::create(ConstraintSystem &cs,
 
 bool IgnoreInvalidFunctionBuilderBody::diagnose(const Solution &solution,
                                                 bool asNote) const {
+  switch (Phase) {
+  // Handled below
+  case ErrorInPhase::PreCheck:
+    break;
+  case ErrorInPhase::ConstraintGeneration:
+    return true; // Already diagnosed by `matchFunctionBuilder`.
+  }
+
   auto *S = getAnchor().get<Stmt *>();
 
   class PreCheckWalker : public ASTWalker {
@@ -1580,7 +1588,7 @@ bool IgnoreInvalidFunctionBuilderBody::diagnose(const Solution &solution,
     }
 
     bool diagnosed() const {
-      return Transaction.hasDiagnostics();
+      return Transaction.hasErrors();
     }
   };
 
@@ -1590,8 +1598,8 @@ bool IgnoreInvalidFunctionBuilderBody::diagnose(const Solution &solution,
   return walker.diagnosed();
 }
 
-IgnoreInvalidFunctionBuilderBody *
-IgnoreInvalidFunctionBuilderBody::create(ConstraintSystem &cs,
-                                         ConstraintLocator *locator) {
-  return new (cs.getAllocator()) IgnoreInvalidFunctionBuilderBody(cs, locator);
+IgnoreInvalidFunctionBuilderBody *IgnoreInvalidFunctionBuilderBody::create(
+    ConstraintSystem &cs, ErrorInPhase phase, ConstraintLocator *locator) {
+  return new (cs.getAllocator())
+      IgnoreInvalidFunctionBuilderBody(cs, phase, locator);
 }

lib/Sema/CSFix.h

@@ -1984,9 +1984,17 @@ class AllowKeyPathWithoutComponents final : public ConstraintFix {
 };
 
 class IgnoreInvalidFunctionBuilderBody final : public ConstraintFix {
-  IgnoreInvalidFunctionBuilderBody(ConstraintSystem &cs,
+  enum class ErrorInPhase {
+    PreCheck,
+    ConstraintGeneration,
+  };
+
+  ErrorInPhase Phase;
+
+  IgnoreInvalidFunctionBuilderBody(ConstraintSystem &cs, ErrorInPhase phase,
                                    ConstraintLocator *locator)
-      : ConstraintFix(cs, FixKind::IgnoreInvalidFunctionBuilderBody, locator) {}
+      : ConstraintFix(cs, FixKind::IgnoreInvalidFunctionBuilderBody, locator),
+        Phase(phase) {}
 
 public:
   std::string getName() const override {
@@ -1999,8 +2007,19 @@ class IgnoreInvalidFunctionBuilderBody final : public ConstraintFix {
     return diagnose(*commonFixes.front().first);
   }
 
-  static IgnoreInvalidFunctionBuilderBody *create(ConstraintSystem &cs,
-                                                  ConstraintLocator *locator);
+  static IgnoreInvalidFunctionBuilderBody *
+  duringPreCheck(ConstraintSystem &cs, ConstraintLocator *locator) {
+    return create(cs, ErrorInPhase::PreCheck, locator);
+  }
+
+  static IgnoreInvalidFunctionBuilderBody *
+  duringConstraintGeneration(ConstraintSystem &cs, ConstraintLocator *locator) {
+    return create(cs, ErrorInPhase::ConstraintGeneration, locator);
+  }
+
+private:
+  static IgnoreInvalidFunctionBuilderBody *
+  create(ConstraintSystem &cs, ErrorInPhase phase, ConstraintLocator *locator);
 };
 
 } // end namespace constraints

lib/Sema/ConstraintSystem.h

@@ -2025,6 +2025,13 @@ class ConstraintSystem {
   /// from declared parameters/result and body.
   llvm::MapVector<const ClosureExpr *, FunctionType *> ClosureTypes;
 
+  /// This is a *global* list of all function builder bodies that have
+  /// been determined to be incorrect by failing constraint generation.
+  ///
+  /// Tracking this information is useful to avoid producing duplicate
+  /// diagnostics when function builder has multiple overloads.
+  llvm::SmallDenseSet<AnyFunctionRef> InvalidFunctionBuilderBodies;
+
   /// Maps node types used within all portions of the constraint
   /// system, instead of directly using the types on the
   /// nodes themselves. This allows us to typecheck and

test/Constraints/function_builder_diags.swift

@@ -619,6 +619,14 @@ struct MyView {
     } // expected-error {{expected identifier after '.' expression}}
   }
 
+  @TupleBuilder var invalidCaseWithoutDot: some P {
+    switch Optional.some(1) {
+    case none: 42 // expected-error {{cannot find 'none' in scope}}
+    case .some(let x):
+      0
+    }
+  }
+
   @TupleBuilder var invalidConversion: Int { // expected-error {{cannot convert value of type 'String' to specified type 'Int'}}
     ""
   }