[flang] Refine output file generation

This patch cleans-up the file generation code in Flang's frontend
driver. It improves the layering between
`CompilerInstance::CreateDefaultOutputFile`,
`CompilerInstance::CreateOutputFile` and their various clients.

* Rename `CreateOutputFile` as `CreateOutputFileImpl` and make it
  private. This method is an implementation detail.
* Instead of passing an `std::error_code` out parameter into
  `CreateOutputFileImpl`, have it return Expected<>. This is a bit shorter
  and idiomatic LLVM.
* Make `CreateDefaultOutputFile` (which calls `CreateOutputFileImpl`)
  issue an error when file creation fails. The error code from
  `CreateOutputFileImpl` is used to generate a meaningful diagnostic
  message.
* Remove error reporting from `PrintPreprocessedAction::ExecuteAction`.
  This is only for cases when output file generation fails. This is
  handled in `CreateDefaultOutputFile` instead (see the previous point).
* Inline `AddOutputFile` into its only caller,
  `CreateDefaultOutputFile`.
* Switch from `lvm::buffer_ostream` to `llvm::buffer_unique_ostream>`
  for non-seekable output streams. This simplifies the logic in the driver
  and was introduced for this very reason in [1]
* Moke sure that the diagnostics from the prescanner when running `-E`
  (`PrintPreprocessedAction::ExecuteAction`) are printed before the actual
  output is generated.
* Update comments, add test.

NOTE: This patch relands [2]. As suggested by Michael Kruse in the
post-commit/post-revert review, I've added the following:
```
config.errc_messages = "@LLVM_LIT_ERRC_MESSAGES@"
```
in Flang's `lit.site.cfg.py.in`. This way, `%errc_ENOENT` in
output-paths.f90 gets the correct value on Windows as well as on Linux.

[1] https://reviews.llvm.org/D93260
[2] fd21d1e

Reviewed By: ashermancinelli

Differential Revision: https://reviews.llvm.org/D108390

Author: banach-space

flang/CMakeLists.txt

@@ -72,6 +72,7 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   include(AddLLVM)
   include(HandleLLVMOptions)
   include(VersionFromVCS)
+  include(GetErrcMessages)
 
   if(FLANG_BUILD_NEW_DRIVER)
     include(AddClang)
@@ -136,6 +137,8 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
          "Generate build targets for the Flang unit tests."
          ON)
 
+  get_errc_messages(LLVM_LIT_ERRC_MESSAGES)
+
 #Handle unittests when out-of-tree
 #LLVM_BUILD_MAIN_SRC_DIR - Path to llvm source when out-of-tree.
   set(FLANG_GTEST_AVAIL 0)

flang/include/flang/Frontend/CompilerInstance.h

@@ -63,11 +63,6 @@ class CompilerInstance {
         : filename_(std::move(inputFilename)) {}
   };
 
-  /// Output stream that doesn't support seeking (e.g. terminal, pipe).
-  /// This stream is normally wrapped in buffer_ostream before being passed
-  /// to users (e.g. via CreateOutputFile).
-  std::unique_ptr<llvm::raw_fd_ostream> nonSeekStream_;
-
   /// The list of active output files.
   std::list<OutputFile> outputFiles_;
 
@@ -189,38 +184,34 @@ class CompilerInstance {
   /// @name Output Files
   /// {
 
-  /// Add an output file onto the list of tracked output files.
-  ///
-  /// \param outFile - The output file info.
-  void AddOutputFile(OutputFile &&outFile);
-
   /// Clear the output file list.
   void ClearOutputFiles(bool eraseFiles);
 
   /// Create the default output file (based on the invocation's options) and
   /// add it to the list of tracked output files. If the name of the output
-  /// file is not provided, it is derived from the input file.
+  /// file is not provided, it will be derived from the input file.
   ///
   /// \param binary     The mode to open the file in.
   /// \param baseInput  If the invocation contains no output file name (i.e.
   ///                   outputFile in FrontendOptions is empty), the input path
   ///                   name to use for deriving the output path.
   /// \param extension  The extension to use for output names derived from
   ///                   \p baseInput.
-  /// \return           ostream for the output file or nullptr on error.
+  /// \return           Null on error, ostream for the output file otherwise
   std::unique_ptr<llvm::raw_pwrite_stream> CreateDefaultOutputFile(
       bool binary = true, llvm::StringRef baseInput = "",
       llvm::StringRef extension = "");
 
+private:
   /// Create a new output file
   ///
   /// \param outputPath   The path to the output file.
-  /// \param error [out]  On failure, the error.
   /// \param binary       The mode to open the file in.
-  /// \return             ostream for the output file or nullptr on error.
-  std::unique_ptr<llvm::raw_pwrite_stream> CreateOutputFile(
-      llvm::StringRef outputPath, std::error_code &error, bool binary);
+  /// \return             Null on error, ostream for the output file otherwise
+  llvm::Expected<std::unique_ptr<llvm::raw_pwrite_stream>> CreateOutputFileImpl(
+      llvm::StringRef outputPath, bool binary);
 
+public:
   /// }
   /// @name Construction Utility Methods
   /// {

flang/lib/Frontend/CompilerInstance.cpp

@@ -51,10 +51,6 @@ void CompilerInstance::set_semaOutputStream(
   semaOutputStream_ = ownedSemaOutputStream_.get();
 }
 
-void CompilerInstance::AddOutputFile(OutputFile &&outFile) {
-  outputFiles_.push_back(std::move(outFile));
-}
-
 // Helper method to generate the path of the output file. The following logic
 // applies:
 // 1. If the user specifies the output file via `-o`, then use that (i.e.
@@ -84,48 +80,52 @@ static std::string GetOutputFilePath(llvm::StringRef outputFilename,
 std::unique_ptr<llvm::raw_pwrite_stream>
 CompilerInstance::CreateDefaultOutputFile(
     bool binary, llvm::StringRef baseName, llvm::StringRef extension) {
-  std::string outputPathName;
-  std::error_code ec;
 
   // Get the path of the output file
   std::string outputFilePath =
       GetOutputFilePath(frontendOpts().outputFile, baseName, extension);
 
   // Create the output file
-  std::unique_ptr<llvm::raw_pwrite_stream> os =
-      CreateOutputFile(outputFilePath, ec, binary);
-
-  // Add the file to the list of tracked output files (provided it was created
-  // successfully)
-  if (os)
-    AddOutputFile(OutputFile(outputPathName));
+  llvm::Expected<std::unique_ptr<llvm::raw_pwrite_stream>> os =
+      CreateOutputFileImpl(outputFilePath, binary);
+
+  // If successful, add the file to the list of tracked output files and
+  // return.
+  if (os) {
+    outputFiles_.emplace_back(OutputFile(outputFilePath));
+    return std::move(*os);
+  }
 
-  return os;
+  // If unsuccessful, issue an error and return Null
+  unsigned DiagID = diagnostics().getCustomDiagID(
+      clang::DiagnosticsEngine::Error, "unable to open output file '%0': '%1'");
+  diagnostics().Report(DiagID)
+      << outputFilePath << llvm::errorToErrorCode(os.takeError()).message();
+  return nullptr;
 }
 
-std::unique_ptr<llvm::raw_pwrite_stream> CompilerInstance::CreateOutputFile(
-    llvm::StringRef outputFilePath, std::error_code &error, bool binary) {
+llvm::Expected<std::unique_ptr<llvm::raw_pwrite_stream>>
+CompilerInstance::CreateOutputFileImpl(
+    llvm::StringRef outputFilePath, bool binary) {
 
   // Creates the file descriptor for the output file
   std::unique_ptr<llvm::raw_fd_ostream> os;
-  std::string osFile;
-  if (!os) {
-    osFile = outputFilePath;
-    os.reset(new llvm::raw_fd_ostream(osFile, error,
-        (binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_TextWithCRLF)));
-    if (error)
-      return nullptr;
+
+  std::error_code error;
+  os.reset(new llvm::raw_fd_ostream(outputFilePath, error,
+      (binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_TextWithCRLF)));
+  if (error) {
+    return llvm::errorCodeToError(error);
   }
 
-  // Return the stream corresponding to the output file.
-  // For non-seekable streams, wrap it in llvm::buffer_ostream first.
+  // For seekable streams, just return the stream corresponding to the output
+  // file.
   if (!binary || os->supportsSeeking())
     return std::move(os);
 
-  assert(!nonSeekStream_ && "The non-seek stream has already been set!");
-  auto b = std::make_unique<llvm::buffer_ostream>(*os);
-  nonSeekStream_ = std::move(os);
-  return std::move(b);
+  // For non-seekable streams, we need to wrap the output stream into something
+  // that supports 'pwrite' and takes care of the ownership for us.
+  return std::make_unique<llvm::buffer_unique_ostream>(std::move(os));
 }
 
 void CompilerInstance::ClearOutputFiles(bool eraseFiles) {
@@ -134,7 +134,6 @@ void CompilerInstance::ClearOutputFiles(bool eraseFiles) {
       llvm::sys::fs::remove(of.filename_);
 
   outputFiles_.clear();
-  nonSeekStream_.reset();
 }
 
 bool CompilerInstance::ExecuteAction(FrontendAction &act) {

flang/lib/Frontend/FrontendActions.cpp

@@ -90,23 +90,24 @@ void PrintPreprocessedAction::ExecuteAction() {
         outForPP, !ci.invocation().preprocessorOpts().noLineDirectives);
   }
 
+  // Print diagnostics from the prescanner
+  ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
+
   // If a pre-defined output stream exists, dump the preprocessed content there
   if (!ci.IsOutputStreamNull()) {
     // Send the output to the pre-defined output buffer.
     ci.WriteOutputStream(outForPP.str());
     return;
   }
 
-  // Print diagnostics from the prescanner
-  ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
-
   // Create a file and save the preprocessed output there
-  if (auto os{ci.CreateDefaultOutputFile(
-          /*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName())}) {
-    (*os) << outForPP.str();
-  } else {
-    llvm::errs() << "Unable to create the output file\n";
+  std::unique_ptr<llvm::raw_pwrite_stream> os{ci.CreateDefaultOutputFile(
+      /*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName())};
+  if (!os) {
+    return;
   }
+
+  (*os) << outForPP.str();
 }
 
 void DebugDumpProvenanceAction::ExecuteAction() {

flang/test/Driver/output-paths.f90

@@ -0,0 +1,12 @@
+! Test the diagnostic for cases when the output file cannot be generated
+
+!--------------------------
+! RUN lines
+!--------------------------
+! RUN: not %flang_fc1 -E -o %t.doesnotexist/somename %s 2> %t
+! RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
+
+!-----------------------
+! EXPECTED OUTPUT
+!-----------------------
+! OUTPUTFAIL: error: unable to open output file '{{.*}}doesnotexist{{.}}somename': '[[MSG]]'

flang/test/lit.site.cfg.py.in

@@ -4,6 +4,7 @@ import sys
 
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
+config.errc_messages = "@LLVM_LIT_ERRC_MESSAGES@"
 config.flang_obj_root = "@FLANG_BINARY_DIR@"
 config.flang_src_dir = "@FLANG_SOURCE_DIR@"
 config.flang_tools_dir = "@FLANG_TOOLS_DIR@"