diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -578,11 +578,6 @@ /// @name Output Files /// { - /// addOutputFile - Add an output file onto the list of tracked output files. - /// - /// \param OutFile - The output file info. - void addOutputFile(OutputFile &&OutFile); - /// clearOutputFiles - Clear the output file list. The underlying output /// streams must have been closed beforehand. /// @@ -695,25 +690,29 @@ /// Create the default output file (from the invocation's options) and add it /// to the list of tracked output files. /// - /// The files created by this function always use temporary files to write to - /// their result (that is, the data is written to a temporary file which will - /// atomically replace the target output on success). + /// The files created by this are usually removed on signal, and, depending + /// on FrontendOptions, may also use a temporary file (that is, the data is + /// written to a temporary file which will atomically replace the target + /// output on success). If a client (like libclang) needs to disable + /// RemoveFileOnSignal, temporary files will be forced on. /// /// \return - Null on error. std::unique_ptr createDefaultOutputFile(bool Binary = true, StringRef BaseInput = "", - StringRef Extension = ""); + StringRef Extension = "", + bool RemoveFileOnSignal = true, + bool CreateMissingDirectories = false); - /// Create a new output file and add it to the list of tracked output files, - /// optionally deriving the output path name. + /// Create a new output file, optionally deriving the output path name, and + /// add it to the list of tracked output files. /// /// \return - Null on error. std::unique_ptr createOutputFile(StringRef OutputPath, bool Binary, bool RemoveFileOnSignal, - StringRef BaseInput, StringRef Extension, bool UseTemporary, - bool CreateMissingDirectories = false); + bool UseTemporary, bool CreateMissingDirectories = false); - /// Create a new output file, optionally deriving the output path name. +private: + /// Create a new output file and add it to the list of tracked output files. /// /// If \p OutputPath is empty, then createOutputFile will derive an output /// path location as \p BaseInput, with any suffix removed, and \p Extension @@ -722,10 +721,6 @@ /// renamed to \p OutputPath in the end. /// /// \param OutputPath - If given, the path to the output file. - /// \param Error [out] - On failure, the error. - /// \param BaseInput - If \p OutputPath is empty, the input path name to use - /// for deriving the output path. - /// \param Extension - The extension to use for derived output names. /// \param Binary - The mode to open the file in. /// \param RemoveFileOnSignal - Whether the file should be registered with /// llvm::sys::RemoveFileOnSignal. Note that this is not safe for @@ -734,17 +729,12 @@ /// OutputPath in the end. /// \param CreateMissingDirectories - When \p UseTemporary is true, create /// missing directories in the output path. - /// \param ResultPathName [out] - If given, the result path name will be - /// stored here on success. - /// \param TempPathName [out] - If given, the temporary file path name - /// will be stored here on success. - std::unique_ptr - createOutputFile(StringRef OutputPath, std::error_code &Error, bool Binary, - bool RemoveFileOnSignal, StringRef BaseInput, - StringRef Extension, bool UseTemporary, - bool CreateMissingDirectories, std::string *ResultPathName, - std::string *TempPathName); + Expected> + createOutputFileImpl(StringRef OutputPath, bool Binary, + bool RemoveFileOnSignal, bool UseTemporary, + bool CreateMissingDirectories); +public: std::unique_ptr createNullOutputFile(); /// } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -646,10 +646,6 @@ // Output Files -void CompilerInstance::addOutputFile(OutputFile &&OutFile) { - OutputFiles.push_back(std::move(OutFile)); -} - void CompilerInstance::clearOutputFiles(bool EraseFiles) { for (OutputFile &OF : OutputFiles) { if (!OF.TempFilename.empty()) { @@ -682,10 +678,25 @@ std::unique_ptr CompilerInstance::createDefaultOutputFile(bool Binary, StringRef InFile, - StringRef Extension) { - return createOutputFile(getFrontendOpts().OutputFile, Binary, - /*RemoveFileOnSignal=*/true, InFile, Extension, - getFrontendOpts().UseTemporary); + StringRef Extension, + bool RemoveFileOnSignal, + bool CreateMissingDirectories) { + StringRef OutputPath = getFrontendOpts().OutputFile; + Optional> PathStorage; + if (OutputPath.empty()) { + if (InFile == "-" || Extension.empty()) { + OutputPath = "-"; + } else { + PathStorage.emplace(InFile); + llvm::sys::path::replace_extension(*PathStorage, Extension); + OutputPath = *PathStorage; + } + } + + // Force a temporary file if RemoveFileOnSignal was disabled. + return createOutputFile(OutputPath, Binary, RemoveFileOnSignal, + getFrontendOpts().UseTemporary || !RemoveFileOnSignal, + CreateMissingDirectories); } std::unique_ptr CompilerInstance::createNullOutputFile() { @@ -694,64 +705,40 @@ std::unique_ptr CompilerInstance::createOutputFile(StringRef OutputPath, bool Binary, - bool RemoveFileOnSignal, StringRef InFile, - StringRef Extension, bool UseTemporary, + bool RemoveFileOnSignal, bool UseTemporary, bool CreateMissingDirectories) { - std::string OutputPathName, TempPathName; - std::error_code EC; - std::unique_ptr OS = createOutputFile( - OutputPath, EC, Binary, RemoveFileOnSignal, InFile, Extension, - UseTemporary, CreateMissingDirectories, &OutputPathName, &TempPathName); - if (!OS) { - getDiagnostics().Report(diag::err_fe_unable_to_open_output) << OutputPath - << EC.message(); - return nullptr; - } - - // Add the output file -- but don't try to remove "-", since this means we are - // using stdin. - addOutputFile( - OutputFile((OutputPathName != "-") ? OutputPathName : "", TempPathName)); - - return OS; + Expected> OS = + createOutputFileImpl(OutputPath, Binary, RemoveFileOnSignal, UseTemporary, + CreateMissingDirectories); + if (OS) + return std::move(*OS); + getDiagnostics().Report(diag::err_fe_unable_to_open_output) + << OutputPath << errorToErrorCode(OS.takeError()).message(); + return nullptr; } -std::unique_ptr CompilerInstance::createOutputFile( - StringRef OutputPath, std::error_code &Error, bool Binary, - bool RemoveFileOnSignal, StringRef InFile, StringRef Extension, - bool UseTemporary, bool CreateMissingDirectories, - std::string *ResultPathName, std::string *TempPathName) { +Expected> +CompilerInstance::createOutputFileImpl(StringRef OutputPath, bool Binary, + bool RemoveFileOnSignal, + bool UseTemporary, + bool CreateMissingDirectories) { assert((!CreateMissingDirectories || UseTemporary) && "CreateMissingDirectories is only allowed when using temporary files"); - std::string OutFile, TempFile; - if (!OutputPath.empty()) { - OutFile = std::string(OutputPath); - } else if (InFile == "-") { - OutFile = "-"; - } else if (!Extension.empty()) { - SmallString<128> Path(InFile); - llvm::sys::path::replace_extension(Path, Extension); - OutFile = std::string(Path.str()); - } else { - OutFile = "-"; - } - std::unique_ptr OS; - std::string OSFile; + Optional OSFile; if (UseTemporary) { - if (OutFile == "-") + if (OutputPath == "-") UseTemporary = false; else { llvm::sys::fs::file_status Status; llvm::sys::fs::status(OutputPath, Status); if (llvm::sys::fs::exists(Status)) { // Fail early if we can't write to the final destination. - if (!llvm::sys::fs::can_write(OutputPath)) { - Error = make_error_code(llvm::errc::operation_not_permitted); - return nullptr; - } + if (!llvm::sys::fs::can_write(OutputPath)) + return llvm::errorCodeToError( + make_error_code(llvm::errc::operation_not_permitted)); // Don't use a temporary if the output is a special file. This handles // things like '-o /dev/null' @@ -761,14 +748,15 @@ } } + std::string TempFile; if (UseTemporary) { // Create a temporary file. // Insert -%%%%%%%% before the extension (if any), and because some tools // (noticeable, clang's own GlobalModuleIndex.cpp) glob for build // artifacts, also append .tmp. - StringRef OutputExtension = llvm::sys::path::extension(OutFile); + StringRef OutputExtension = llvm::sys::path::extension(OutputPath); SmallString<128> TempPath = - StringRef(OutFile).drop_back(OutputExtension.size()); + StringRef(OutputPath).drop_back(OutputExtension.size()); TempPath += "-%%%%%%%%"; TempPath += OutputExtension; TempPath += ".tmp"; @@ -795,22 +783,23 @@ } if (!OS) { - OSFile = OutFile; + OSFile = OutputPath; + std::error_code EC; OS.reset(new llvm::raw_fd_ostream( - OSFile, Error, + *OSFile, EC, (Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text))); - if (Error) - return nullptr; + if (EC) + return llvm::errorCodeToError(EC); } // Make sure the out stream file gets removed if we crash. if (RemoveFileOnSignal) - llvm::sys::RemoveFileOnSignal(OSFile); + llvm::sys::RemoveFileOnSignal(*OSFile); - if (ResultPathName) - *ResultPathName = OutFile; - if (TempPathName) - *TempPathName = TempFile; + // Add the output file -- but don't try to remove "-", since this means we are + // using stdin. + OutputFiles.emplace_back(((OutputPath != "-") ? OutputPath : "").str(), + std::move(TempFile)); if (!Binary || OS->supportsSeeking()) return std::move(OS); diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -136,13 +136,9 @@ std::unique_ptr GeneratePCHAction::CreateOutputFile(CompilerInstance &CI, StringRef InFile, std::string &OutputFile) { - // We use createOutputFile here because this is exposed via libclang, and we - // must disable the RemoveFileOnSignal behavior. - // We use a temporary to avoid race conditions. - std::unique_ptr OS = - CI.createOutputFile(CI.getFrontendOpts().OutputFile, /*Binary=*/true, - /*RemoveFileOnSignal=*/false, InFile, - /*Extension=*/"", CI.getFrontendOpts().UseTemporary); + // Because this is exposed via libclang we must disable RemoveFileOnSignal. + std::unique_ptr OS = CI.createDefaultOutputFile( + /*Binary=*/true, InFile, /*Extension=*/"", /*RemoveFileOnSignal=*/false); if (!OS) return nullptr; @@ -219,13 +215,10 @@ ModuleMapFile); } - // We use createOutputFile here because this is exposed via libclang, and we - // must disable the RemoveFileOnSignal behavior. - // We use a temporary to avoid race conditions. - return CI.createOutputFile(CI.getFrontendOpts().OutputFile, /*Binary=*/true, - /*RemoveFileOnSignal=*/false, InFile, - /*Extension=*/"", /*UseTemporary=*/true, - /*CreateMissingDirectories=*/true); + // Because this is exposed via libclang we must disable RemoveFileOnSignal. + return CI.createDefaultOutputFile(/*Binary=*/true, InFile, /*Extension=*/"", + /*RemoveFileOnSignal=*/false, + /*CreateMissingDirectories=*/true); } bool GenerateModuleInterfaceAction::BeginSourceFileAction( diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp --- a/clang/tools/driver/cc1_main.cpp +++ b/clang/tools/driver/cc1_main.cpp @@ -248,13 +248,9 @@ if (llvm::timeTraceProfilerEnabled()) { SmallString<128> Path(Clang->getFrontendOpts().OutputFile); llvm::sys::path::replace_extension(Path, "json"); - if (auto profilerOutput = - Clang->createOutputFile(Path.str(), - /*Binary=*/false, - /*RemoveFileOnSignal=*/false, "", - /*Extension=*/"json", - /*useTemporary=*/false)) { - + if (auto profilerOutput = Clang->createOutputFile( + Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false, + /*useTemporary=*/false)) { llvm::timeTraceProfilerWrite(*profilerOutput); // FIXME(ibiryukov): make profilerOutput flush in destructor instead. profilerOutput->flush();