[flang-commits] [flang] 787c443 - [flang] Refine output file generation
Andrzej Warzynski via flang-commits
flang-commits at lists.llvm.org
Sat Aug 21 08:25:29 PDT 2021
Author: Andrzej Warzynski
Date: 2021-08-21T15:18:48Z
New Revision: 787c443a8da7add81d8ff7d430afcac16a1e4b0c
URL: https://github.com/llvm/llvm-project/commit/787c443a8da7add81d8ff7d430afcac16a1e4b0c
DIFF: https://github.com/llvm/llvm-project/commit/787c443a8da7add81d8ff7d430afcac16a1e4b0c.diff
LOG: [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] fd21d1e198e381a2b9e7af1701044462b2d386cd
Reviewed By: ashermancinelli
Differential Revision: https://reviews.llvm.org/D108390
Added:
flang/test/Driver/output-paths.f90
Modified:
flang/CMakeLists.txt
flang/include/flang/Frontend/CompilerInstance.h
flang/lib/Frontend/CompilerInstance.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/test/lit.site.cfg.py.in
Removed:
################################################################################
diff --git a/flang/CMakeLists.txt b/flang/CMakeLists.txt
index f17816ee29b82..f5e602b6b8441 100644
--- a/flang/CMakeLists.txt
+++ b/flang/CMakeLists.txt
@@ -72,6 +72,7 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
include(AddLLVM)
include(HandleLLVMOptions)
include(VersionFromVCS)
+ include(GetErrcMessages)
include(AddClang)
@@ -132,6 +133,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)
diff --git a/flang/include/flang/Frontend/CompilerInstance.h b/flang/include/flang/Frontend/CompilerInstance.h
index 557c626ad9385..2871fdcadf222 100644
--- a/flang/include/flang/Frontend/CompilerInstance.h
+++ b/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,17 +184,12 @@ 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.
@@ -207,20 +197,21 @@ class CompilerInstance {
/// 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
/// {
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index 4c6a875420654..20a9b58a5a3c1 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/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) {
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index f5ff2095281cd..b43cf086bcfce 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -90,6 +90,9 @@ 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.
@@ -97,16 +100,14 @@ 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
- 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() {
diff --git a/flang/test/Driver/output-paths.f90 b/flang/test/Driver/output-paths.f90
new file mode 100644
index 0000000000000..394425fb4310a
--- /dev/null
+++ b/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]]'
diff --git a/flang/test/lit.site.cfg.py.in b/flang/test/lit.site.cfg.py.in
index 5314be8502f6b..378602dd71568 100644
--- a/flang/test/lit.site.cfg.py.in
+++ b/flang/test/lit.site.cfg.py.in
@@ -6,6 +6,7 @@ config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
config.llvm_shlib_dir = path(r"@SHLIBDIR@")
config.llvm_plugin_ext = "@LLVM_PLUGIN_EXT@"
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@"
More information about the flang-commits
mailing list