[FileSystem] Split up the OpenFlags enumeration.

This breaks the OpenFlags enumeration into two separate
enumerations: OpenFlags and CreationDisposition.  The first
controls the behavior of the API depending on whether or not
the target file already exists, and is not a flags-based
enum.  The second controls more flags-like values.

This yields a more easy to understand API, while also allowing
flags to be passed to the openForRead api, where most of the
values didn't make sense before.  This also makes the apis more
testable as it becomes easy to enumerate all the configurations
which make sense, so I've added many new tests to exercise all
the different values.

llvm-svn: 334221

clang-tools-extra/clang-move/tool/ClangMoveMain.cpp

@@ -30,8 +30,8 @@ namespace {
 
 std::error_code CreateNewFile(const llvm::Twine &path) {
   int fd = 0;
-  if (std::error_code ec =
-          llvm::sys::fs::openFileForWrite(path, fd, llvm::sys::fs::F_Text))
+  if (std::error_code ec = llvm::sys::fs::openFileForWrite(
+          path, fd, llvm::sys::fs::CD_CreateAlways, llvm::sys::fs::F_Text))
     return ec;
 
   return llvm::sys::Process::SafelyCloseFileDescriptor(fd);

clang-tools-extra/clangd/tool/ClangdMain.cpp

@@ -159,7 +159,8 @@ int main(int argc, char *argv[]) {
   llvm::Optional<llvm::raw_fd_ostream> InputMirrorStream;
   if (!InputMirrorFile.empty()) {
     std::error_code EC;
-    InputMirrorStream.emplace(InputMirrorFile, /*ref*/ EC, llvm::sys::fs::F_RW);
+    InputMirrorStream.emplace(InputMirrorFile, /*ref*/ EC,
+                              llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);
     if (EC) {
       InputMirrorStream.reset();
       llvm::errs() << "Error while opening an input mirror file: "
@@ -174,7 +175,8 @@ int main(int argc, char *argv[]) {
   std::unique_ptr<trace::EventTracer> Tracer;
   if (auto *TraceFile = getenv("CLANGD_TRACE")) {
     std::error_code EC;
-    TraceStream.emplace(TraceFile, /*ref*/ EC, llvm::sys::fs::F_RW);
+    TraceStream.emplace(TraceFile, /*ref*/ EC,
+                        llvm::sys::fs::FA_Read | llvm::sys::fs::FA_Write);
     if (EC) {
       TraceStream.reset();
       llvm::errs() << "Error while opening trace file " << TraceFile << ": "

clang/lib/Basic/VirtualFileSystem.cpp

@@ -258,7 +258,8 @@ ErrorOr<std::unique_ptr<File>>
 RealFileSystem::openFileForRead(const Twine &Name) {
   int FD;
   SmallString<256> RealName;
-  if (std::error_code EC = sys::fs::openFileForRead(Name, FD, &RealName))
+  if (std::error_code EC =
+          sys::fs::openFileForRead(Name, FD, sys::fs::OF_None, &RealName))
     return EC;
   return std::unique_ptr<File>(new RealFile(FD, Name.str(), RealName.str()));
 }

clang/lib/Driver/Driver.cpp

@@ -1293,7 +1293,7 @@ void Driver::generateCompilationDiagnostics(
 
   std::string Script = CrashInfo.Filename.rsplit('.').first.str() + ".sh";
   std::error_code EC;
-  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::F_Excl);
+  llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew);
   if (EC) {
     Diag(clang::diag::note_drv_command_failed_diag_msg)
         << "Error generating run script: " + Script + " " + EC.message();

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

@@ -238,10 +238,8 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
                    << "-" << i << ".html";
           llvm::sys::path::append(Model, Directory,
                                   filename.str());
-          EC = llvm::sys::fs::openFileForWrite(Model,
-                                               FD,
-                                               llvm::sys::fs::F_RW |
-                                               llvm::sys::fs::F_Excl);
+          EC = llvm::sys::fs::openFileForReadWrite(
+              Model, FD, llvm::sys::fs::CD_CreateNew, llvm::sys::fs::OF_None);
           if (EC && EC != llvm::errc::file_exists) {
               llvm::errs() << "warning: could not create file '" << Model
                            << "': " << EC.message() << '\n';

lldb/source/Core/Debugger.cpp

@@ -1241,8 +1241,8 @@ bool Debugger::EnableLog(llvm::StringRef channel,
       if (log_options & LLDB_LOG_OPTION_APPEND)
         flags |= llvm::sys::fs::F_Append;
       int FD;
-      if (std::error_code ec =
-              llvm::sys::fs::openFileForWrite(log_file, FD, flags)) {
+      if (std::error_code ec = llvm::sys::fs::openFileForWrite(
+              log_file, FD, llvm::sys::fs::CD_CreateAlways, flags)) {
         error_stream << "Unable to open log file: " << ec.message();
         return false;
       }

llvm/include/llvm/Support/FileSystem.h

@@ -676,32 +676,48 @@ bool status_known(const basic_file_status &s);
 ///          platform-specific error_code.
 std::error_code status_known(const Twine &path, bool &result);
 
-enum OpenFlags : unsigned {
-  F_None = 0,
-
-  /// F_Excl - When opening a file, this flag makes raw_fd_ostream
-  /// report an error if the file already exists.
-  F_Excl = 1,
+enum CreationDisposition : unsigned {
+  /// CD_CreateAlways - When opening a file:
+  ///   * If it already exists, truncate it.
+  ///   * If it does not already exist, create a new file.
+  CD_CreateAlways = 0,
+
+  /// CD_CreateNew - When opening a file:
+  ///   * If it already exists, fail.
+  ///   * If it does not already exist, create a new file.
+  CD_CreateNew = 1,
+
+  /// CD_OpenAlways - When opening a file:
+  ///   * If it already exists, open the file with the offset set to 0.
+  ///   * If it does not already exist, fail.
+  CD_OpenExisting = 2,
+
+  /// CD_OpenAlways - When opening a file:
+  ///   * If it already exists, open the file with the offset set to 0.
+  ///   * If it does not already exist, create a new file.
+  CD_OpenAlways = 3,
+};
 
-  /// F_Append - When opening a file, if it already exists append to the
-  /// existing file instead of returning an error.  This may not be specified
-  /// with F_Excl.
-  F_Append = 2,
+enum FileAccess : unsigned {
+  FA_Read = 1,
+  FA_Write = 2,
+};
 
-  /// F_NoTrunc - When opening a file, if it already exists don't truncate
-  /// the file contents.  F_Append implies F_NoTrunc, but F_Append seeks to
-  /// the end of the file, which F_NoTrunc doesn't.
-  F_NoTrunc = 4,
+enum OpenFlags : unsigned {
+  OF_None = 0,
+  F_None = 0, // For compatibility
 
   /// The file should be opened in text mode on platforms that make this
   /// distinction.
-  F_Text = 8,
+  OF_Text = 1,
+  F_Text = 1, // For compatibility
 
-  /// Open the file for read and write.
-  F_RW = 16,
+  /// The file should be opened in append mode.
+  OF_Append = 2,
+  F_Append = 2, // For compatibility
 
   /// Delete the file on close. Only makes a difference on windows.
-  F_Delete = 32
+  OF_Delete = 4
 };
 
 /// Create a uniquely named file.
@@ -824,6 +840,15 @@ inline OpenFlags &operator|=(OpenFlags &A, OpenFlags B) {
   return A;
 }
 
+inline FileAccess operator|(FileAccess A, FileAccess B) {
+  return FileAccess(unsigned(A) | unsigned(B));
+}
+
+inline FileAccess &operator|=(FileAccess &A, FileAccess B) {
+  A = A | B;
+  return A;
+}
+
 /// @brief Opens the file with the given name in a write-only or read-write
 /// mode, returning its open file descriptor. If the file does not exist, it
 /// is created.
@@ -840,7 +865,45 @@ inline OpenFlags &operator|=(OpenFlags &A, OpenFlags B) {
 /// @returns errc::success if \a Name has been opened, otherwise a
 ///          platform-specific error_code.
 std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
-                                 OpenFlags Flags, unsigned Mode = 0666);
+                                 CreationDisposition Disp = CD_CreateAlways,
+                                 OpenFlags Flags = OF_None,
+                                 unsigned Mode = 0666);
+
+/// @brief Opens the file with the given name in a write-only or read-write
+/// mode, returning its open file descriptor. If the file does not exist, it
+/// is created.
+///
+/// The caller is responsible for closing the freeing the file once they are
+/// finished with it.
+///
+/// @param Name The path of the file to open, relative or absolute.
+/// @param Flags Additional flags used to determine whether the file should be
+///              opened in, for example, read-write or in write-only mode.
+/// @param Mode The access permissions of the file, represented in octal.
+/// @returns a platform-specific file descriptor if \a Name has been opened,
+///          otherwise an error object.
+Expected<file_t> openNativeFileForWrite(const Twine &Name,
+                                        CreationDisposition Disp,
+                                        OpenFlags Flags, unsigned Mode = 0666);
+
+/// @brief Opens the file with the given name in a write-only or read-write
+/// mode, returning its open file descriptor. If the file does not exist, it
+/// is created.
+///
+/// The caller is responsible for closing the file descriptor once they are
+/// finished with it.
+///
+/// @param Name The path of the file to open, relative or absolute.
+/// @param ResultFD If the file could be opened successfully, its descriptor
+///                 is stored in this location. Otherwise, this is set to -1.
+/// @param Flags Additional flags used to determine whether the file should be
+///              opened in, for example, read-write or in write-only mode.
+/// @param Mode The access permissions of the file, represented in octal.
+/// @returns errc::success if \a Name has been opened, otherwise a
+///          platform-specific error_code.
+std::error_code openFileForReadWrite(const Twine &Name, int &ResultFD,
+                                     CreationDisposition Disp, OpenFlags Flags,
+                                     unsigned Mode = 0666);
 
 /// @brief Opens the file with the given name in a write-only or read-write
 /// mode, returning its open file descriptor. If the file does not exist, it
@@ -855,8 +918,10 @@ std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
 /// @param Mode The access permissions of the file, represented in octal.
 /// @returns a platform-specific file descriptor if \a Name has been opened,
 ///          otherwise an error object.
-Expected<file_t> openNativeFileForWrite(const Twine &Name, OpenFlags Flags,
-                                        unsigned Mode = 0666);
+Expected<file_t> openNativeFileForReadWrite(const Twine &Name,
+                                            CreationDisposition Disp,
+                                            OpenFlags Flags,
+                                            unsigned Mode = 0666);
 
 /// @brief Opens the file with the given name in a read-only mode, returning
 /// its open file descriptor.
@@ -873,6 +938,7 @@ Expected<file_t> openNativeFileForWrite(const Twine &Name, OpenFlags Flags,
 /// @returns errc::success if \a Name has been opened, otherwise a
 ///          platform-specific error_code.
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
+                                OpenFlags Flags = OF_None,
                                 SmallVectorImpl<char> *RealPath = nullptr);
 
 /// @brief Opens the file with the given name in a read-only mode, returning
@@ -888,7 +954,7 @@ std::error_code openFileForRead(const Twine &Name, int &ResultFD,
 /// @returns a platform-specific file descriptor if \a Name has been opened,
 ///          otherwise an error object.
 Expected<file_t>
-openNativeFileForRead(const Twine &Name,
+openNativeFileForRead(const Twine &Name, OpenFlags Flags = OF_None,
                       SmallVectorImpl<char> *RealPath = nullptr);
 
 /// @brief Close the file object.  This should be used instead of ::close for

llvm/include/llvm/Support/raw_ostream.h

@@ -33,7 +33,9 @@ class FormattedBytes;
 
 namespace sys {
 namespace fs {
+enum FileAccess : unsigned;
 enum OpenFlags : unsigned;
+enum CreationDisposition : unsigned;
 } // end namespace fs
 } // end namespace sys
 
@@ -397,7 +399,15 @@ class raw_fd_ostream : public raw_pwrite_stream {
   /// As a special case, if Filename is "-", then the stream will use
   /// STDOUT_FILENO instead of opening a file. This will not close the stdout
   /// descriptor.
+  raw_fd_ostream(StringRef Filename, std::error_code &EC);
   raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                 sys::fs::CreationDisposition Disp);
+  raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                 sys::fs::FileAccess Access);
+  raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                 sys::fs::OpenFlags Flags);
+  raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                 sys::fs::CreationDisposition Disp, sys::fs::FileAccess Access,
                  sys::fs::OpenFlags Flags);
 
   /// FD is the file descriptor that this writes to.  If ShouldClose is true,

llvm/lib/Support/FileOutputBuffer.cpp

@@ -82,9 +82,10 @@ class InMemoryBuffer : public FileOutputBuffer {
   size_t getBufferSize() const override { return Buffer.size(); }
 
   Error commit() override {
+    using namespace sys::fs;
     int FD;
     std::error_code EC;
-    if (auto EC = openFileForWrite(FinalPath, FD, fs::F_None, Mode))
+    if (auto EC = openFileForWrite(FinalPath, FD, CD_CreateAlways, OF_None))
       return errorCodeToError(EC);
     raw_fd_ostream OS(FD, /*shouldClose=*/true, /*unbuffered=*/true);
     OS << StringRef((const char *)Buffer.base(), Buffer.size());

llvm/lib/Support/MemoryBuffer.cpp

@@ -244,7 +244,7 @@ static ErrorOr<std::unique_ptr<MB>>
 getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize,
            uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile) {
   int FD;
-  std::error_code EC = sys::fs::openFileForRead(Filename, FD);
+  std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None);
 
   if (EC)
     return EC;
@@ -364,8 +364,8 @@ static ErrorOr<std::unique_ptr<WriteThroughMemoryBuffer>>
 getReadWriteFile(const Twine &Filename, uint64_t FileSize, uint64_t MapSize,
                  uint64_t Offset) {
   int FD;
-  std::error_code EC = sys::fs::openFileForWrite(
-      Filename, FD, sys::fs::F_RW | sys::fs::F_NoTrunc);
+  std::error_code EC = sys::fs::openFileForReadWrite(
+      Filename, FD, sys::fs::CD_OpenExisting, sys::fs::OF_None);
 
   if (EC)
     return EC;
@@ -518,7 +518,7 @@ ErrorOr<std::unique_ptr<MemoryBuffer>> MemoryBuffer::getSTDIN() {
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFileAsStream(const Twine &Filename) {
   int FD;
-  std::error_code EC = sys::fs::openFileForRead(Filename, FD);
+  std::error_code EC = sys::fs::openFileForRead(Filename, FD, sys::fs::OF_None);
   if (EC)
     return EC;
   ErrorOr<std::unique_ptr<MemoryBuffer>> Ret =

llvm/lib/Support/Path.cpp

@@ -169,7 +169,7 @@ static std::error_code
 createUniqueEntity(const Twine &Model, int &ResultFD,
                    SmallVectorImpl<char> &ResultPath, bool MakeAbsolute,
                    unsigned Mode, FSEntity Type,
-                   sys::fs::OpenFlags Flags = sys::fs::F_RW) {
+                   sys::fs::OpenFlags Flags = sys::fs::OF_None) {
   SmallString<128> ModelStorage;
   Model.toVector(ModelStorage);
 
@@ -201,8 +201,8 @@ createUniqueEntity(const Twine &Model, int &ResultFD,
   switch (Type) {
   case FS_File: {
     if (std::error_code EC =
-            sys::fs::openFileForWrite(Twine(ResultPath.begin()), ResultFD,
-                                      Flags | sys::fs::F_Excl, Mode)) {
+            sys::fs::openFileForReadWrite(Twine(ResultPath.begin()), ResultFD,
+                                          sys::fs::CD_CreateNew, Flags, Mode)) {
       if (EC == errc::file_exists)
         goto retry_random_path;
       return EC;
@@ -929,9 +929,10 @@ std::error_code create_directories(const Twine &Path, bool IgnoreExisting,
 
 std::error_code copy_file(const Twine &From, const Twine &To) {
   int ReadFD, WriteFD;
-  if (std::error_code EC = openFileForRead(From, ReadFD))
+  if (std::error_code EC = openFileForRead(From, ReadFD, OF_None))
     return EC;
-  if (std::error_code EC = openFileForWrite(To, WriteFD, F_None)) {
+  if (std::error_code EC =
+          openFileForWrite(To, WriteFD, CD_CreateAlways, OF_None)) {
     close(ReadFD);
     return EC;
   }
@@ -983,7 +984,7 @@ ErrorOr<MD5::MD5Result> md5_contents(int FD) {
 
 ErrorOr<MD5::MD5Result> md5_contents(const Twine &Path) {
   int FD;
-  if (auto EC = openFileForRead(Path, FD))
+  if (auto EC = openFileForRead(Path, FD, OF_None))
     return EC;
 
   auto Result = md5_contents(FD);
@@ -1180,7 +1181,7 @@ Expected<TempFile> TempFile::create(const Twine &Model, unsigned Mode) {
   int FD;
   SmallString<128> ResultPath;
   if (std::error_code EC =
-          createUniqueFile(Model, FD, ResultPath, Mode, F_Delete | F_RW))
+          createUniqueFile(Model, FD, ResultPath, Mode, OF_Delete))
     return errorCodeToError(EC);
 
   TempFile Ret(ResultPath, FD);

llvm/lib/Support/TarWriter.cpp

@@ -159,8 +159,10 @@ static void writeUstarHeader(raw_fd_ostream &OS, StringRef Prefix,
 // Creates a TarWriter instance and returns it.
 Expected<std::unique_ptr<TarWriter>> TarWriter::create(StringRef OutputPath,
                                                        StringRef BaseDir) {
+  using namespace sys::fs;
   int FD;
-  if (std::error_code EC = openFileForWrite(OutputPath, FD, sys::fs::F_None))
+  if (std::error_code EC =
+          openFileForWrite(OutputPath, FD, CD_CreateAlways, OF_None))
     return make_error<StringError>("cannot open " + OutputPath, EC);
   return std::unique_ptr<TarWriter>(new TarWriter(FD, BaseDir));
 }