Address review feedback

Author: cachemeifyoucan

include/swift/AST/ASTContext.h

@@ -285,7 +285,7 @@ class ASTContext final {
   DiagnosticEngine &Diags;
 
   /// OutputBackend for writing outputs.
-  llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend;
 
   /// If the shared pointer is not a \c nullptr and the pointee is \c true,
   /// all operations working on this ASTContext should be aborted at the next
@@ -1525,13 +1525,13 @@ class ASTContext final {
   /// Get the output backend. The output backend needs to be initialized via
   /// constructor or `setOutputBackend`.
   llvm::vfs::OutputBackend &getOutputBackend() const {
-    assert(Backend && "OutputBackend is not setup");
-    return *Backend;
+    assert(OutputBackend && "OutputBackend is not setup");
+    return *OutputBackend;
   }
   /// Set output backend for virtualized outputs.
   void setOutputBackend(
-      llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend) {
-    Backend = std::move(OutputBackend);
+      llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutBackend) {
+    OutputBackend = std::move(OutBackend);
   }
 
 private:

include/swift/AST/DiagnosticsCommon.def

@@ -29,6 +29,9 @@ ERROR(not_implemented,none,
 ERROR(error_opening_output,none,
       "error opening '%0' for output: %1", (StringRef, StringRef))
 
+ERROR(error_closing_output,none,
+      "error closing '%0' for output: %1", (StringRef, StringRef))
+
 ERROR(cannot_find_group_info_file,none,
       "cannot find group info file at path: '%0'", (StringRef))
 

include/swift/AST/FileSystem.h

@@ -26,7 +26,7 @@ namespace swift {
 /// \returns true if there were any errors, either from the filesystem
 /// operations or from \p action returning true.
 inline bool
-withOutputFile(DiagnosticEngine &diags, llvm::vfs::OutputBackend &Backend,
+withOutputPath(DiagnosticEngine &diags, llvm::vfs::OutputBackend &Backend,
                StringRef outputPath,
                llvm::function_ref<bool(llvm::raw_pwrite_stream &)> action) {
   assert(!outputPath.empty());
@@ -37,15 +37,19 @@ withOutputFile(DiagnosticEngine &diags, llvm::vfs::OutputBackend &Backend,
   if (!outputFile) {
     diags.diagnose(SourceLoc(), diag::error_opening_output, outputPath,
                    toString(outputFile.takeError()));
-    return false;
+    return true;
   }
 
   bool failed = action(*outputFile);
   // If there is an error, discard output. Otherwise keep the output file.
   if (auto error = failed ? outputFile->discard() : outputFile->keep()) {
-    diags.diagnose(SourceLoc(), diag::error_opening_output, outputPath,
-                   toString(std::move(error)));
-    return false;
+    // Don't diagnose discard error.
+    if (failed)
+      consumeError(std::move(error));
+    else
+      diags.diagnose(SourceLoc(), diag::error_closing_output, outputPath,
+                     toString(std::move(error)));
+    return true;
   }
   return failed;
 }

include/swift/AST/IRGenOptions.h

@@ -318,7 +318,7 @@ class IRGenOptions {
   /// Print the LLVM inline tree at the end of the LLVM pass pipeline.
   unsigned PrintInlineTree : 1;
 
-  /// Always recompile the output even the module hash might match.
+  /// Always recompile the output even if the module hash might match.
   unsigned AlwaysCompile : 1;
 
   /// Whether we should embed the bitcode file.

include/swift/ClangImporter/ClangImporter.h

@@ -395,9 +395,7 @@ class ClangImporter final : public ClangModuleLoader {
   /// replica.
   ///
   /// \sa clang::GeneratePCHAction
-  bool
-  emitBridgingPCH(llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
-                  StringRef headerPath, StringRef outputPCHPath);
+  bool emitBridgingPCH(StringRef headerPath, StringRef outputPCHPath);
 
   /// Returns true if a clang CompilerInstance can successfully read in a PCH,
   /// assuming it exists, with the current options. This can be used to find out
@@ -408,21 +406,16 @@ class ClangImporter final : public ClangModuleLoader {
   /// module map into the replica and emits a PCM file for one of the modules it
   /// declares. Delegates to clang for everything except construction of the
   /// replica.
-  bool emitPrecompiledModule(
-      llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
-      StringRef moduleMapPath, StringRef moduleName, StringRef outputPath);
+  bool emitPrecompiledModule(StringRef moduleMapPath, StringRef moduleName,
+                             StringRef outputPath);
 
   /// Makes a temporary replica of the ClangImporter's CompilerInstance and
   /// dumps information about a PCM file (assumed to be generated by -emit-pcm
   /// or in the Swift module cache). Delegates to clang for everything except
   /// construction of the replica.
-  bool dumpPrecompiledModule(
-      llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
-      StringRef modulePath, StringRef outputPath);
+  bool dumpPrecompiledModule(StringRef modulePath, StringRef outputPath);
 
-  bool
-  runPreprocessor(llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> backend,
-                  StringRef inputPath, StringRef outputPath);
+  bool runPreprocessor(StringRef inputPath, StringRef outputPath);
   const clang::Module *getClangOwningModule(ClangNode Node) const;
   bool hasTypedef(const clang::Decl *typeDecl) const;
 
@@ -510,8 +503,7 @@ class ClangImporter final : public ClangModuleLoader {
 
   Optional<std::string>
   getOrCreatePCH(const ClangImporterOptions &ImporterOptions,
-                 StringRef SwiftPCHHash,
-                 llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend);
+                 StringRef SwiftPCHHash);
   Optional<std::string>
   /// \param isExplicit true if the PCH filename was passed directly
   /// with -import-objc-header option.

include/swift/Frontend/Frontend.h

@@ -461,7 +461,7 @@ class CompilerInstance {
   std::unique_ptr<UnifiedStatsReporter> Stats;
 
   /// Virtual OutputBackend.
-  llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> TheOutputBackend = nullptr;
+  llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend = nullptr;
 
   /// The verification output backend.
   using HashBackendTy = llvm::vfs::HashingOutputBackend<llvm::BLAKE3>;
@@ -518,11 +518,11 @@ class CompilerInstance {
   }
 
   llvm::vfs::OutputBackend &getOutputBackend() const {
-    return *TheOutputBackend;
+    return *OutputBackend;
   }
   void
   setOutputBackend(llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> Backend) {
-    TheOutputBackend = std::move(Backend);
+    OutputBackend = std::move(Backend);
   }
   using HashingBackendPtrTy = llvm::IntrusiveRefCntPtr<HashBackendTy>;
   HashingBackendPtrTy getHashingBackend() { return HashBackend; }

include/swift/Option/FrontendOptions.td

@@ -1176,9 +1176,9 @@ def enable_emit_generic_class_ro_t_list :
   HelpText<"Enable emission of a section with references to class_ro_t of "
            "generic class patterns">;
 
-def enable_swift_deterministic_check :
-  Flag<["-"], "enable-swift-deterministic-check">,
-  HelpText<"Check swift compiler output determinisim by run it twice">;
+def enable_deterministic_check :
+  Flag<["-"], "enable-deterministic-check">,
+  HelpText<"Check compiler output determinisim by running it twice">;
 def always_compile_output_files :
   Flag<["-"], "always-compile-output-files">,
   HelpText<"Always compile output files even it might not change the results">;

lib/AST/ASTContext.cpp

@@ -661,12 +661,12 @@ ASTContext::ASTContext(
     ClangImporterOptions &ClangImporterOpts,
     symbolgraphgen::SymbolGraphOptions &SymbolGraphOpts,
     SourceManager &SourceMgr, DiagnosticEngine &Diags,
-    llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutputBackend,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::OutputBackend> OutBackend,
     std::function<bool(llvm::StringRef, bool)> PreModuleImportCallback)
     : LangOpts(langOpts), TypeCheckerOpts(typecheckOpts), SILOpts(silOpts),
       SearchPathOpts(SearchPathOpts), ClangImporterOpts(ClangImporterOpts),
       SymbolGraphOpts(SymbolGraphOpts), SourceMgr(SourceMgr), Diags(Diags),
-      Backend(std::move(OutputBackend)), evaluator(Diags, langOpts),
+      OutputBackend(std::move(OutBackend)), evaluator(Diags, langOpts),
       TheBuiltinModule(createBuiltinModule(*this)),
       StdlibModuleName(getIdentifier(STDLIB_NAME)),
       SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),
@@ -714,8 +714,8 @@ ASTContext::ASTContext(
 
   createModuleToExecutablePluginMap();
   // Provide a default OnDiskOutputBackend if user didn't supply one.
-  if (!Backend)
-    Backend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
+  if (!OutputBackend)
+    OutputBackend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
 }
 
 ASTContext::~ASTContext() {

lib/AST/FineGrainedDependencies.cpp

@@ -377,7 +377,7 @@ void SourceFileDepGraph::emitDotFile(llvm::vfs::OutputBackend &outputBackend,
                                      StringRef outputPath,
                                      DiagnosticEngine &diags) {
   std::string dotFileName = outputPath.str() + ".dot";
-  withOutputFile(
+  withOutputPath(
       diags, outputBackend, dotFileName, [&](llvm::raw_pwrite_stream &out) {
         DotFileEmitter<SourceFileDepGraph>(out, *this, false, false).emit();
         return false;

lib/AST/FineGrainedDependencyFormat.cpp

@@ -502,7 +502,7 @@ bool swift::fine_grained_dependencies::writeFineGrainedDependencyGraphToPath(
     DiagnosticEngine &diags, llvm::vfs::OutputBackend &backend, StringRef path,
     const SourceFileDepGraph &g) {
   PrettyStackTraceStringAction stackTrace("saving fine-grained dependency graph", path);
-  return withOutputFile(diags, backend, path, [&](llvm::raw_ostream &out) {
+  return withOutputPath(diags, backend, path, [&](llvm::raw_ostream &out) {
     SmallVector<char, 0> Buffer;
     llvm::BitstreamWriter Writer{Buffer};
     writeFineGrainedDependencyGraph(Writer, g, Purpose::ForSwiftDeps);