[flang-commits] [flang] 316be03 - Revert "[flang] Refine output file generation"

Andrzej Warzynski via flang-commits flang-commits at lists.llvm.org
Fri Aug 20 02:16:23 PDT 2021


Author: Andrzej Warzynski
Date: 2021-08-20T09:16:11Z
New Revision: 316be03ff5968d589fda054d5f03319400b22b38

URL: https://github.com/llvm/llvm-project/commit/316be03ff5968d589fda054d5f03319400b22b38
DIFF: https://github.com/llvm/llvm-project/commit/316be03ff5968d589fda054d5f03319400b22b38.diff

LOG: Revert "[flang] Refine output file generation"

This reverts commit fd21d1e198e381a2b9e7af1701044462b2d386cd.

The test added in this patch [1] is failing on Windows and causing the
Windows BuildBot [2] to fail. I don't see any obvious way to fix this,
so reverting in order to investigate.

[1] llvm-project/flang/test/Driver/output-paths.f90
[2] https://lab.llvm.org/buildbot/#/builders/172/builds/2077

Added: 
    

Modified: 
    flang/include/flang/Frontend/CompilerInstance.h
    flang/lib/Frontend/CompilerInstance.cpp
    flang/lib/Frontend/FrontendActions.cpp

Removed: 
    flang/test/Driver/output-paths.f90


################################################################################
diff  --git a/flang/include/flang/Frontend/CompilerInstance.h b/flang/include/flang/Frontend/CompilerInstance.h
index 2871fdcadf222..557c626ad9385 100644
--- a/flang/include/flang/Frontend/CompilerInstance.h
+++ b/flang/include/flang/Frontend/CompilerInstance.h
@@ -63,6 +63,11 @@ 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_;
 
@@ -184,12 +189,17 @@ 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 will be derived from the input file.
+  /// file is not provided, it is 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.
@@ -197,21 +207,20 @@ class CompilerInstance {
   ///                   name to use for deriving the output path.
   /// \param extension  The extension to use for output names derived from
   ///                   \p baseInput.
-  /// \return           Null on error, ostream for the output file otherwise
+  /// \return           ostream for the output file or nullptr on error.
   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             Null on error, ostream for the output file otherwise
-  llvm::Expected<std::unique_ptr<llvm::raw_pwrite_stream>> CreateOutputFileImpl(
-      llvm::StringRef outputPath, bool binary);
+  /// \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);
 
-public:
   /// }
   /// @name Construction Utility Methods
   /// {

diff  --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index 8fb2a32501a83..4c6a875420654 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -51,6 +51,10 @@ 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.
@@ -80,51 +84,48 @@ 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
-  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);
-  }
+  std::unique_ptr<llvm::raw_pwrite_stream> os =
+      CreateOutputFile(outputFilePath, ec, binary);
 
-  // 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;
+  // Add the file to the list of tracked output files (provided it was created
+  // successfully)
+  if (os)
+    AddOutputFile(OutputFile(outputPathName));
+
+  return os;
 }
 
-llvm::Expected<std::unique_ptr<llvm::raw_pwrite_stream>>
-CompilerInstance::CreateOutputFileImpl(
-    llvm::StringRef outputFilePath, bool binary) {
+std::unique_ptr<llvm::raw_pwrite_stream> CompilerInstance::CreateOutputFile(
+    llvm::StringRef outputFilePath, std::error_code &error, 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);
-
-  // For seekable streams, just return the stream corresponding to the output
-  // file.
+  // Return the stream corresponding to the output file.
+  // For non-seekable streams, wrap it in llvm::buffer_ostream first.
   if (!binary || os->supportsSeeking())
     return std::move(os);
 
-  // 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));
+  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);
 }
 
 void CompilerInstance::ClearOutputFiles(bool eraseFiles) {
@@ -133,6 +134,7 @@ void CompilerInstance::ClearOutputFiles(bool eraseFiles) {
       llvm::sys::fs::remove(of.filename_);
 
   outputFiles_.clear();
+  nonSeekStream_.reset();
 }
 
 bool CompilerInstance::ExecuteAction(FrontendAction &act) {

diff  --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index b43cf086bcfce..f5ff2095281cd 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -90,9 +90,6 @@ 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.
@@ -100,14 +97,16 @@ void PrintPreprocessedAction::ExecuteAction() {
     return;
   }
 
+  // Print diagnostics from the prescanner
+  ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
+
   // Create a file and save the preprocessed output there
-  std::unique_ptr<llvm::raw_pwrite_stream> os{ci.CreateDefaultOutputFile(
-      /*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName())};
-  if (!os) {
-    return;
+  if (auto os{ci.CreateDefaultOutputFile(
+          /*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName())}) {
+    (*os) << outForPP.str();
+  } else {
+    llvm::errs() << "Unable to create the output file\n";
   }
-
-  (*os) << outForPP.str();
 }
 
 void DebugDumpProvenanceAction::ExecuteAction() {

diff  --git a/flang/test/Driver/output-paths.f90 b/flang/test/Driver/output-paths.f90
deleted file mode 100644
index 394425fb4310a..0000000000000
--- a/flang/test/Driver/output-paths.f90
+++ /dev/null
@@ -1,12 +0,0 @@
-! 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]]'


        


More information about the flang-commits mailing list