[Frontend] Ensure constraint solving runs salvaging

When index-while-building is enabled, system modules are rebuilt from
their interface with diagnostics silenced so failures are not propagated
to the build. This is enabled via a local diagnostic engine that has no
consumers.

Typechecking uses the lack of consumers to add
`ConstraintSystemFlags::SuppressDiagnostics`, which controls whether
salvaging (and output of diagnostics) is run. There are cases today
where salvaging can find a correct solution though, so we should ensure
that it's always run.

This is a quick workaround for the indexing case - we should instead
always run salvaging, regardless of whether diagnostics are suppressed
or not.

Resolves rdar://117133297.

Author: bnbarham

lib/Frontend/ModuleInterfaceBuilder.cpp

@@ -329,12 +329,14 @@ bool ImplicitModuleInterfaceBuilder::buildSwiftModuleInternal(
       llvm::RestorePrettyStackState(savedInnerPrettyStackState);
     };
 
+    NullDiagnosticConsumer noopConsumer;
     llvm::Optional<DiagnosticEngine> localDiags;
     DiagnosticEngine *rebuildDiags = diags;
     if (silenceInterfaceDiagnostics) {
       // To silence diagnostics, use a local temporary engine.
       localDiags.emplace(sourceMgr);
       rebuildDiags = &*localDiags;
+      rebuildDiags->addConsumer(noopConsumer);
     }
 
     SubError = (bool)subASTDelegate.runInSubCompilerInstance(

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -2007,8 +2007,13 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
   subInstance.getSourceMgr().setFileSystem(SM.getFileSystem());
 
   ForwardingDiagnosticConsumer FDC(*Diags);
-  if (!silenceErrors)
+  NullDiagnosticConsumer noopConsumer;
+  if (!silenceErrors) {
     subInstance.addDiagnosticConsumer(&FDC);
+  } else {
+    subInstance.addDiagnosticConsumer(&noopConsumer);
+  }
+
   std::string InstanceSetupError;
   if (subInstance.setup(subInvocation, InstanceSetupError)) {
     return std::make_error_code(std::errc::not_supported);

test/Index/index_system_modules_swiftinterfaces.swift

@@ -113,7 +113,7 @@
 /// Index a client reading from a broken swiftinterface
 // RUN: %empty-directory(%t/idx)
 // RUN: %empty-directory(%t/modulecache)
-// RUN: echo "breaking_the_swifinterface" >> %t/SDK/Frameworks/SystemModule.framework/Modules/SystemModule.swiftmodule/%module-target-triple.swiftinterface
+// RUN: echo "breaking_the_swiftinterface" >> %t/SDK/Frameworks/SystemModule.framework/Modules/SystemModule.swiftmodule/%module-target-triple.swiftinterface
 
 // RUN: %target-swift-frontend -typecheck -parse-stdlib \
 // RUN:     -index-system-modules \
@@ -128,7 +128,7 @@
 
 /// We don't expect to see the swiftinterface error for indexing
 // BROKEN-BUILD-NOT: error
-// BROKEN-BUILD-NOT: breaking_the_swifinterface
+// BROKEN-BUILD-NOT: breaking_the_swiftinterface
 // BROKEN-BUILD: indexing system module {{.*}} skipping
 
 /// We don't expect SystemModule to be indexed with a broken swiftinterface
@@ -178,6 +178,19 @@ import SystemDepB
 public func systemFunc() {}
 func leakyFunc(_ a: SecretType) {}
 
+// Currently requires salvaging, which we need to make sure runs when the
+// interface is rebuilt (as it produces a solution), we'll crash if it isn't.
+public struct SysA { public init() {} }
+public struct SysB { public init() {} }
+@available(macOS, unavailable)
+public func forceDisjunction() -> SysA { return SysA() }
+public func forceDisjunction() -> SysB { return SysB() }
+@available(macOS, unavailable)
+@inlinable
+public func requireSalvage() -> SysA {
+  return forceDisjunction()
+}
+
 //--- SystemDepA.swift
 import SystemDepCommon
 public func systemDepAFunc() {}