Merge pull request #83974 from hamishknight/reprap

[Sema] Avoid marking TypeRepr invalid with `SilenceErrors`

Author: hamishknight

include/swift/Sema/ConstraintSystem.h

@@ -5736,7 +5736,7 @@ class ConstraintSystem {
 
   /// If we aren't certain that we've emitted a diagnostic, emit a fallback
   /// diagnostic.
-  void maybeProduceFallbackDiagnostic(SyntacticElementTarget target) const;
+  void maybeProduceFallbackDiagnostic(SourceLoc loc) const;
 
   /// Check whether given AST node represents an argument of an application
   /// of some sort (call, operator invocation, subscript etc.)

lib/Sema/CSApply.cpp

@@ -10023,7 +10023,7 @@ ConstraintSystem::applySolution(Solution &solution,
   {
     const auto &score = solution.getFixedScore();
     if (score.Data[SK_Fix] > numResolvableFixes || score.Data[SK_Hole] > 0) {
-      maybeProduceFallbackDiagnostic(target);
+      maybeProduceFallbackDiagnostic(target.getLoc());
       return std::nullopt;
     }
   }

lib/Sema/CSDiagnostics.cpp

@@ -236,6 +236,11 @@ bool FailureDiagnostic::conformsToKnownProtocol(
   return TypeChecker::conformsToKnownProtocol(type, protocol);
 }
 
+bool FallbackDiagnostic::diagnoseAsError() {
+  getConstraintSystem().maybeProduceFallbackDiagnostic(getLoc());
+  return true;
+}
+
 Type RequirementFailure::getOwnerType() const {
   auto anchor = getAnchor();
   // If diagnostic is anchored at assignment expression

lib/Sema/CSDiagnostics.h

@@ -210,6 +210,15 @@ class FailureDiagnostic {
                                  llvm::SmallVectorImpl<char> &scratch) const;
 };
 
+/// Emits a fallback diagnostic message if no other error has been emitted.
+class FallbackDiagnostic final : public FailureDiagnostic {
+public:
+  FallbackDiagnostic(const Solution &solution, ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Base class for all of the diagnostics related to generic requirement
 /// failures, provides common information like failed requirement,
 /// declaration where such requirement comes from, etc.

lib/Sema/CSFix.cpp

@@ -2187,7 +2187,10 @@ AllowKeyPathWithoutComponents::create(ConstraintSystem &cs,
 
 bool IgnoreInvalidResultBuilderBody::diagnose(const Solution &solution,
                                               bool asNote) const {
-  return true; // Already diagnosed by `matchResultBuilder`.
+  // This should already be diagnosed by `matchResultBuilder`, emit a fallback
+  // diagnostic if not.
+  FallbackDiagnostic diag(solution, getLocator());
+  return diag.diagnose(asNote);
 }
 
 IgnoreInvalidResultBuilderBody *
@@ -2198,7 +2201,10 @@ IgnoreInvalidResultBuilderBody::create(ConstraintSystem &cs,
 
 bool IgnoreInvalidASTNode::diagnose(const Solution &solution,
                                     bool asNote) const {
-  return true; // Already diagnosed by the producer of ErrorExpr or ErrorType.
+  // This should already be diagnosed by the producer of ErrorExpr or ErrorType,
+  // emit a fallback diagnostic if not.
+  FallbackDiagnostic diag(solution, getLocator());
+  return diag.diagnose(asNote);
 }
 
 IgnoreInvalidASTNode *IgnoreInvalidASTNode::create(ConstraintSystem &cs,

lib/Sema/CSSolver.cpp

@@ -1457,7 +1457,7 @@ ConstraintSystem::solve(SyntacticElementTarget &target,
     }
 
     case SolutionResult::Error:
-      maybeProduceFallbackDiagnostic(target);
+      maybeProduceFallbackDiagnostic(target.getLoc());
       return std::nullopt;
 
     case SolutionResult::TooComplex: {

lib/Sema/ConstraintSystem.cpp

@@ -4875,8 +4875,7 @@ bool ConstraintSystem::isConformanceUnavailable(ProtocolConformanceRef conforman
 
 /// If we aren't certain that we've emitted a diagnostic, emit a fallback
 /// diagnostic.
-void ConstraintSystem::maybeProduceFallbackDiagnostic(
-    SyntacticElementTarget target) const {
+void ConstraintSystem::maybeProduceFallbackDiagnostic(SourceLoc loc) const {
   if (Options.contains(ConstraintSystemFlags::SuppressDiagnostics))
     return;
 
@@ -4888,7 +4887,7 @@ void ConstraintSystem::maybeProduceFallbackDiagnostic(
       (diagnosticTransaction && diagnosticTransaction->hasErrors()))
     return;
 
-  ctx.Diags.diagnose(target.getLoc(), diag::failed_to_produce_diagnostic);
+  ctx.Diags.diagnose(loc, diag::failed_to_produce_diagnostic);
 }
 
 SourceLoc constraints::getLoc(ASTNode anchor) {

lib/Sema/TypeCheckType.cpp

@@ -2637,10 +2637,12 @@ static Type evaluateTypeResolution(const TypeResolution *resolution,
       TypeResolver(*resolution, silContext)
           .resolveType(TyR, resolution->getOptions());
 
-  // If we resolved down to an error, make sure to mark the typeRepr as invalid
-  // so we don't produce a redundant diagnostic.
+  // If we resolved down to an error, and we haven't silenced diagnostics, make
+  // sure to mark the typeRepr as invalid so we don't produce a redundant
+  // diagnostic.
   if (result->hasError()) {
-    TyR->setInvalid();
+    if (!options.contains(TypeResolutionFlags::SilenceErrors))
+      TyR->setInvalid();
     return result;
   }
 

test/Constraints/patterns.swift

@@ -820,3 +820,17 @@ do {
     }
   }
 }
+
+// Make sure we diagnose 'Undefined' here.
+func testUndefinedTypeInPattern(_ x: Int) {
+  switch x {
+  case Optional<Undefined>.alsoUndefined: // expected-error {{cannot find type 'Undefined' in scope}}
+    break
+  }
+  _ = {
+    switch x {
+    case Optional<Undefined>.alsoUndefined: // expected-error {{cannot find type 'Undefined' in scope}}
+      break
+    }
+  }
+}

validation-test/compiler_crashers_2/75d8fd688b445bb.swift renamed to validation-test/compiler_crashers_2_fixed/75d8fd688b445bb.swift

@@ -1,5 +1,5 @@
 // {"kind":"typecheck","original":"b7a10f72","signature":"(anonymous namespace)::Verifier::walkToExprPre(swift::Expr*)","signatureAssert":"Assertion failed: ((HadError || !isa<SourceFile *>(M) || cast<SourceFile *>(M)->ASTStage < SourceFile::TypeChecked) && \"OverloadedDeclRef\" \"in wrong phase\"), function walkToExprPre"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 func a(b : Int) {
   _ = {
     switch b {