[llvm] r334221 - [FileSystem] Split up the OpenFlags enumeration.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 7 12:58:58 PDT 2018


Author: zturner
Date: Thu Jun  7 12:58:58 2018
New Revision: 334221

URL: http://llvm.org/viewvc/llvm-project?rev=334221&view=rev
Log:
[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.

Modified:
    llvm/trunk/include/llvm/Support/FileSystem.h
    llvm/trunk/include/llvm/Support/raw_ostream.h
    llvm/trunk/lib/Support/FileOutputBuffer.cpp
    llvm/trunk/lib/Support/MemoryBuffer.cpp
    llvm/trunk/lib/Support/Path.cpp
    llvm/trunk/lib/Support/TarWriter.cpp
    llvm/trunk/lib/Support/Unix/Path.inc
    llvm/trunk/lib/Support/Windows/Path.inc
    llvm/trunk/lib/Support/Windows/Program.inc
    llvm/trunk/lib/Support/raw_ostream.cpp
    llvm/trunk/tools/llvm-ar/llvm-ar.cpp
    llvm/trunk/tools/llvm-cov/SourceCoverageView.cpp
    llvm/trunk/tools/llvm-cov/TestingSupport.cpp
    llvm/trunk/tools/llvm-exegesis/lib/BenchmarkResult.cpp
    llvm/trunk/tools/llvm-exegesis/llvm-exegesis.cpp
    llvm/trunk/tools/llvm-rc/llvm-rc.cpp
    llvm/trunk/unittests/Support/LockFileManagerTest.cpp
    llvm/trunk/unittests/Support/Path.cpp
    llvm/trunk/unittests/Support/ReplaceFileTest.cpp
    llvm/trunk/unittests/Support/raw_pwrite_stream_test.cpp

Modified: llvm/trunk/include/llvm/Support/FileSystem.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/FileSystem.h?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/FileSystem.h (original)
+++ llvm/trunk/include/llvm/Support/FileSystem.h Thu Jun  7 12:58:58 2018
@@ -676,32 +676,48 @@ bool status_known(const basic_file_statu
 ///          platform-specific error_code.
 std::error_code status_known(const Twine &path, bool &result);
 
-enum OpenFlags : unsigned {
-  F_None = 0,
+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,
+};
+
+enum FileAccess : unsigned {
+  FA_Read = 1,
+  FA_Write = 2,
+};
 
-  /// F_Excl - When opening a file, this flag makes raw_fd_ostream
-  /// report an error if the file already exists.
-  F_Excl = 1,
-
-  /// 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,
-
-  /// 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 &
   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 &
 /// @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 T
 /// @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(
 /// @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 Tw
 /// @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

Modified: llvm/trunk/include/llvm/Support/raw_ostream.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/raw_ostream.h?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/raw_ostream.h (original)
+++ llvm/trunk/include/llvm/Support/raw_ostream.h Thu Jun  7 12:58:58 2018
@@ -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 @@ public:
   /// 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,

Modified: llvm/trunk/lib/Support/FileOutputBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FileOutputBuffer.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/FileOutputBuffer.cpp (original)
+++ llvm/trunk/lib/Support/FileOutputBuffer.cpp Thu Jun  7 12:58:58 2018
@@ -82,9 +82,10 @@ public:
   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());

Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
+++ llvm/trunk/lib/Support/MemoryBuffer.cpp Thu Jun  7 12:58:58 2018
@@ -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<WriteThro
 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>> M
 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 =

Modified: llvm/trunk/lib/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Path.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Path.cpp (original)
+++ llvm/trunk/lib/Support/Path.cpp Thu Jun  7 12:58:58 2018
@@ -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 @@ retry_random_path:
   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
 
 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
 
 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(cons
   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);

Modified: llvm/trunk/lib/Support/TarWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/TarWriter.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/TarWriter.cpp (original)
+++ llvm/trunk/lib/Support/TarWriter.cpp Thu Jun  7 12:58:58 2018
@@ -159,8 +159,10 @@ static void writeUstarHeader(raw_fd_ostr
 // 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));
 }

Modified: llvm/trunk/lib/Support/Unix/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Path.inc (original)
+++ llvm/trunk/lib/Support/Unix/Path.inc Thu Jun  7 12:58:58 2018
@@ -722,21 +722,69 @@ static bool hasProcSelfFD() {
 }
 #endif
 
-std::error_code openFileForRead(const Twine &Name, int &ResultFD,
-                                SmallVectorImpl<char> *RealPath) {
-  SmallString<128> Storage;
-  StringRef P = Name.toNullTerminatedStringRef(Storage);
-  int OpenFlags = O_RDONLY;
+static int nativeOpenFlags(CreationDisposition Disp, OpenFlags Flags,
+                           FileAccess Access) {
+  int Result = 0;
+  if (Access == FA_Read)
+    Result |= O_RDONLY;
+  else if (Access == FA_Write)
+    Result |= O_WRONLY;
+  else if (Access == (FA_Read | FA_Write))
+    Result |= O_RDWR;
+
+  // This is for compatibility with old code that assumed F_Append implied
+  // would open an existing file.  See Windows/Path.inc for a longer comment.
+  if (Flags & F_Append)
+    Disp = CD_OpenAlways;
+
+  if (Disp == CD_CreateNew) {
+    Result |= O_CREAT; // Create if it doesn't exist.
+    Result |= O_EXCL;  // Fail if it does.
+  } else if (Disp == CD_CreateAlways) {
+    Result |= O_CREAT; // Create if it doesn't exist.
+    Result |= O_TRUNC; // Truncate if it does.
+  } else if (Disp == CD_OpenAlways) {
+    Result |= O_CREAT; // Create if it doesn't exist.
+  } else if (Disp == CD_OpenExisting) {
+    // Nothing special, just don't add O_CREAT and we get these semantics.
+  }
+
+  if (Flags & F_Append)
+    Result |= O_APPEND;
+
 #ifdef O_CLOEXEC
-  OpenFlags |= O_CLOEXEC;
+  Result |= O_CLOEXEC;
 #endif
-  if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags)) < 0)
+
+  return Result;
+}
+
+static std::error_code openFile(const Twine &Name, int &ResultFD,
+                                CreationDisposition Disp, FileAccess Access,
+                                OpenFlags Flags, unsigned Mode) {
+  int OpenFlags = nativeOpenFlags(Disp, Flags, Access);
+
+  SmallString<128> Storage;
+  StringRef P = Name.toNullTerminatedStringRef(Storage);
+  if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags, Mode)) <
+      0)
     return std::error_code(errno, std::generic_category());
 #ifndef O_CLOEXEC
   int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
   (void)r;
   assert(r == 0 && "fcntl(F_SETFD, FD_CLOEXEC) failed");
 #endif
+  return std::error_code();
+}
+
+std::error_code openFileForRead(const Twine &Name, int &ResultFD,
+                                OpenFlags Flags,
+                                SmallVectorImpl<char> *RealPath) {
+  std::error_code EC =
+      openFile(Name, ResultFD, CD_OpenExisting, FA_Read, Flags, 0666);
+  if (EC)
+    return EC;
+
   // Attempt to get the real name of the file, if the user asked
   if(!RealPath)
     return std::error_code();
@@ -756,6 +804,9 @@ std::error_code openFileForRead(const Tw
     if (CharCount > 0)
       RealPath->append(Buffer, Buffer + CharCount);
   } else {
+    SmallString<128> Storage;
+    StringRef P = Name.toNullTerminatedStringRef(Storage);
+
     // Use ::realpath to get the real path name
     if (::realpath(P.begin(), Buffer) != nullptr)
       RealPath->append(Buffer, Buffer + strlen(Buffer));
@@ -764,56 +815,42 @@ std::error_code openFileForRead(const Tw
   return std::error_code();
 }
 
-Expected<file_t> openNativeFileForRead(const Twine &Name,
+Expected<file_t> openNativeFileForRead(const Twine &Name, OpenFlags Flags,
                                        SmallVectorImpl<char> *RealPath) {
   file_t ResultFD;
-  std::error_code EC = openFileForRead(Name, ResultFD, RealPath);
+  std::error_code EC = openFileForRead(Name, ResultFD, Flags, RealPath);
   if (EC)
     return errorCodeToError(EC);
   return ResultFD;
 }
 
 std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
-                            sys::fs::OpenFlags Flags, unsigned Mode) {
-  // Verify that we don't have both "append" and "excl".
-  assert((!(Flags & sys::fs::F_Excl) || !(Flags & sys::fs::F_Append)) &&
-         "Cannot specify both 'excl' and 'append' file creation flags!");
-
-  int OpenFlags = O_CREAT;
-
-#ifdef O_CLOEXEC
-  OpenFlags |= O_CLOEXEC;
-#endif
-
-  if (Flags & F_RW)
-    OpenFlags |= O_RDWR;
-  else
-    OpenFlags |= O_WRONLY;
-
-  if (Flags & F_Append)
-    OpenFlags |= O_APPEND;
-  else if (!(Flags & F_NoTrunc))
-    OpenFlags |= O_TRUNC;
+                                 CreationDisposition Disp, OpenFlags Flags,
+                                 unsigned Mode) {
+  return openFile(Name, ResultFD, Disp, FA_Write, Flags, Mode);
+}
 
-  if (Flags & F_Excl)
-    OpenFlags |= O_EXCL;
+Expected<file_t> openNativeFileForWrite(const Twine &Name,
+                                        CreationDisposition Disp,
+                                        OpenFlags Flags, unsigned Mode) {
+  file_t ResultFD;
+  std::error_code EC = openFileForWrite(Name, ResultFD, Disp, Flags, Mode);
+  if (EC)
+    return errorCodeToError(EC);
+  return ResultFD;
+}
 
-  SmallString<128> Storage;
-  StringRef P = Name.toNullTerminatedStringRef(Storage);
-  if ((ResultFD = sys::RetryAfterSignal(-1, open, P.begin(), OpenFlags, Mode)) < 0)
-    return std::error_code(errno, std::generic_category());
-#ifndef O_CLOEXEC
-  int r = fcntl(ResultFD, F_SETFD, FD_CLOEXEC);
-  (void)r;
-  assert(r == 0 && "fcntl(F_SETFD, FD_CLOEXEC) failed");
-#endif
-  return std::error_code();
+std::error_code openFileForReadWrite(const Twine &Name, int &ResultFD,
+                                     CreationDisposition Disp, OpenFlags Flags,
+                                     unsigned Mode) {
+  return openFile(Name, ResultFD, Disp, FA_Read | FA_Write, Flags, Mode);
 }
 
-Expected<file_t> openNativeFileForWrite(const Twine &Name, OpenFlags Flags,
-                                        unsigned Mode) {
+Expected<file_t> openNativeFileForReadWrite(const Twine &Name,
+                                            CreationDisposition Disp,
+                                            OpenFlags Flags, unsigned Mode) {
   file_t ResultFD;
-  std::error_code EC = openFileForWrite(Name, ResultFD, Flags, Mode);
+  std::error_code EC = openFileForReadWrite(Name, ResultFD, Disp, Flags, Mode);
   if (EC)
     return errorCodeToError(EC);
   return ResultFD;

Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Thu Jun  7 12:58:58 2018
@@ -1035,31 +1035,20 @@ ErrorOr<basic_file_status> directory_ent
   return Status;
 }
 
-static std::error_code directoryRealPath(const Twine &Name,
-                                         SmallVectorImpl<char> &RealPath) {
-  SmallVector<wchar_t, 128> PathUTF16;
+static std::error_code nativeFileToFd(Expected<HANDLE> H, int &ResultFD,
+                                      OpenFlags Flags) {
+  int CrtOpenFlags = 0;
+  if (Flags & OF_Append)
+    CrtOpenFlags |= _O_APPEND;
 
-  if (std::error_code EC = widenPath(Name, PathUTF16))
-    return EC;
+  if (Flags & OF_Text)
+    CrtOpenFlags |= _O_TEXT;
 
-  HANDLE H =
-      ::CreateFileW(PathUTF16.begin(), GENERIC_READ,
-                    FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
-                    NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
-  if (H == INVALID_HANDLE_VALUE)
-    return mapWindowsError(GetLastError());
-  std::error_code EC = realPathFromHandle(H, RealPath);
-  ::CloseHandle(H);
-  return EC;
-}
-
-static std::error_code nativeFileToFd(Expected<HANDLE> H, int &ResultFD,
-                                      int OpenFlags) {
   ResultFD = -1;
   if (!H)
     return errorToErrorCode(H.takeError());
 
-  ResultFD = ::_open_osfhandle(intptr_t(*H), OpenFlags);
+  ResultFD = ::_open_osfhandle(intptr_t(*H), CrtOpenFlags);
   if (ResultFD == -1) {
     ::CloseHandle(*H);
     return mapWindowsError(ERROR_INVALID_HANDLE);
@@ -1067,23 +1056,62 @@ static std::error_code nativeFileToFd(Ex
   return std::error_code();
 }
 
-std::error_code openFileForRead(const Twine &Name, int &ResultFD,
-                                SmallVectorImpl<char> *RealPath) {
-  Expected<HANDLE> NativeFile = openNativeFileForRead(Name, RealPath);
-  return nativeFileToFd(std::move(NativeFile), ResultFD, 0);
+static DWORD nativeOpenFlags(OpenFlags Flags) {
+  DWORD Result = 0;
+  if (Flags & OF_Delete)
+    Result |= FILE_FLAG_DELETE_ON_CLOSE;
+
+  if (Result == 0)
+    Result = FILE_ATTRIBUTE_NORMAL;
+  return Result;
+}
+
+static DWORD nativeDisposition(CreationDisposition Disp, OpenFlags Flags) {
+  // This is a compatibility hack.  Really we should respect the creation
+  // disposition, but a lot of old code relied on the implicit assumption that
+  // OF_Append implied it would open an existing file.  Since the disposition is
+  // now explicit and defaults to CD_CreateAlways, this assumption would cause
+  // any usage of OF_Append to append to a new file, even if the file already
+  // existed.  A better solution might have two new creation dispositions:
+  // CD_AppendAlways and CD_AppendNew.  This would also address the problem of
+  // OF_Append being used on a read-only descriptor, which doesn't make sense.
+  if (Flags & OF_Append)
+    return OPEN_ALWAYS;
+
+  switch (Disp) {
+  case CD_CreateAlways:
+    return CREATE_ALWAYS;
+  case CD_CreateNew:
+    return CREATE_NEW;
+  case CD_OpenAlways:
+    return OPEN_ALWAYS;
+  case CD_OpenExisting:
+    return OPEN_EXISTING;
+  }
+  llvm_unreachable("unreachable!");
+}
+
+static DWORD nativeAccess(FileAccess Access, OpenFlags Flags) {
+  DWORD Result = 0;
+  if (Access & FA_Read)
+    Result |= GENERIC_READ;
+  if (Access & FA_Write)
+    Result |= GENERIC_WRITE;
+  if (Flags & OF_Delete)
+    Result |= DELETE;
+  return Result;
 }
 
-Expected<file_t> openNativeFileForRead(const Twine &Name,
-                                       SmallVectorImpl<char> *RealPath) {
+static Expected<file_t> nativeOpenFile(const Twine &Name, DWORD Disp,
+                                       DWORD Access, DWORD Flags) {
   SmallVector<wchar_t, 128> PathUTF16;
-
   if (std::error_code EC = widenPath(Name, PathUTF16))
     return errorCodeToError(EC);
 
   HANDLE H =
-      ::CreateFileW(PathUTF16.begin(), GENERIC_READ,
+      ::CreateFileW(PathUTF16.begin(), Access,
                     FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
-                    NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+                    NULL, Disp, Flags, NULL);
   if (H == INVALID_HANDLE_VALUE) {
     DWORD LastError = ::GetLastError();
     std::error_code EC = mapWindowsError(LastError);
@@ -1096,74 +1124,84 @@ Expected<file_t> openNativeFileForRead(c
       return errorCodeToError(make_error_code(errc::is_a_directory));
     return errorCodeToError(EC);
   }
+  return H;
+}
 
-  // Fetch the real name of the file, if the user asked
-  if (RealPath)
-    realPathFromHandle(H, *RealPath);
+static Expected<file_t> openFile(const Twine &Name, CreationDisposition Disp,
+                                 FileAccess Access, OpenFlags Flags) {
+  // Verify that we don't have both "append" and "excl".
+  assert((!(Disp == CD_CreateNew) || !(Flags & OF_Append)) &&
+         "Cannot specify both 'CreateNew' and 'Append' file creation flags!");
 
-  return H;
+  DWORD NativeFlags = nativeOpenFlags(Flags);
+  DWORD NativeDisp = nativeDisposition(Disp, Flags);
+  DWORD NativeAccess = nativeAccess(Access, Flags);
+
+  return nativeOpenFile(Name, NativeDisp, NativeAccess, NativeFlags);
 }
 
-std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
-                            sys::fs::OpenFlags Flags, unsigned Mode) {
-  int OpenFlags = 0;
-  if (Flags & F_Append)
-    OpenFlags |= _O_APPEND;
+static std::error_code directoryRealPath(const Twine &Name,
+                                         SmallVectorImpl<char> &RealPath) {
+  Expected<file_t> EF = nativeOpenFile(Name, OPEN_EXISTING, GENERIC_READ,
+                                       FILE_FLAG_BACKUP_SEMANTICS);
+  if (!EF)
+    return errorToErrorCode(EF.takeError());
 
-  if (Flags & F_Text)
-    OpenFlags |= _O_TEXT;
+  std::error_code EC = realPathFromHandle(*EF, RealPath);
+  ::CloseHandle(*EF);
+  return EC;
+}
 
-  Expected<HANDLE> NativeFile = openNativeFileForWrite(Name, Flags, Mode);
-  return nativeFileToFd(std::move(NativeFile), ResultFD, OpenFlags);
+std::error_code openFileForRead(const Twine &Name, int &ResultFD,
+                                OpenFlags Flags,
+                                SmallVectorImpl<char> *RealPath) {
+  Expected<HANDLE> NativeFile = openNativeFileForRead(Name, Flags, RealPath);
+  return nativeFileToFd(std::move(NativeFile), ResultFD, OF_None);
 }
 
-Expected<file_t> openNativeFileForWrite(const Twine &Name, OpenFlags Flags,
-                                        unsigned Mode) {
-  // Verify that we don't have both "append" and "excl".
-  assert((!(Flags & sys::fs::F_Excl) || !(Flags & sys::fs::F_Append)) &&
-         "Cannot specify both 'excl' and 'append' file creation flags!");
+Expected<file_t> openNativeFileForRead(const Twine &Name, OpenFlags Flags,
+                                       SmallVectorImpl<char> *RealPath) {
 
-  SmallVector<wchar_t, 128> PathUTF16;
+  Expected<file_t> Result = openFile(Name, CD_OpenExisting, FA_Read, Flags);
 
-  if (std::error_code EC = widenPath(Name, PathUTF16))
-    return errorCodeToError(EC);
+  // Fetch the real name of the file, if the user asked
+  if (Result && RealPath)
+    realPathFromHandle(*Result, *RealPath);
 
-  DWORD CreationDisposition;
-  if (Flags & F_Excl)
-    CreationDisposition = CREATE_NEW;
-  else if ((Flags & F_Append) || (Flags & F_NoTrunc))
-    CreationDisposition = OPEN_ALWAYS;
-  else
-    CreationDisposition = CREATE_ALWAYS;
-
-  DWORD Access = GENERIC_WRITE;
-  DWORD Attributes = FILE_ATTRIBUTE_NORMAL;
-  if (Flags & F_RW)
-    Access |= GENERIC_READ;
-  if (Flags & F_Delete) {
-    Access |= DELETE;
-    Attributes |= FILE_FLAG_DELETE_ON_CLOSE;
-  }
+  return std::move(Result);
+}
 
-  HANDLE H =
-      ::CreateFileW(PathUTF16.data(), Access,
-                    FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
-                    NULL, CreationDisposition, Attributes, NULL);
+std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
+                                 CreationDisposition Disp, OpenFlags Flags,
+                                 unsigned Mode) {
+  Expected<HANDLE> NativeFile = openNativeFileForWrite(Name, Disp, Flags, Mode);
+  if (!NativeFile)
+    return errorToErrorCode(NativeFile.takeError());
 
-  if (H == INVALID_HANDLE_VALUE) {
-    DWORD LastError = ::GetLastError();
-    std::error_code EC = mapWindowsError(LastError);
-    // Provide a better error message when trying to open directories.
-    // This only runs if we failed to open the file, so there is probably
-    // no performances issues.
-    if (LastError != ERROR_ACCESS_DENIED)
-      return errorCodeToError(EC);
-    if (is_directory(Name))
-      return errorCodeToError(make_error_code(errc::is_a_directory));
-    return errorCodeToError(EC);
-  }
+  return nativeFileToFd(std::move(NativeFile), ResultFD, Flags);
+}
 
-  return H;
+Expected<file_t> openNativeFileForWrite(const Twine &Name,
+                                        CreationDisposition Disp,
+                                        OpenFlags Flags, unsigned Mode) {
+  return openFile(Name, Disp, FA_Write, Flags);
+}
+
+std::error_code openFileForReadWrite(const Twine &Name, int &ResultFD,
+                                     CreationDisposition Disp, OpenFlags Flags,
+                                     unsigned Mode) {
+  Expected<HANDLE> NativeFile =
+      openNativeFileForReadWrite(Name, Disp, Flags, Mode);
+  if (!NativeFile)
+    return errorToErrorCode(NativeFile.takeError());
+
+  return nativeFileToFd(std::move(NativeFile), ResultFD, Flags);
+}
+
+Expected<file_t> openNativeFileForReadWrite(const Twine &Name,
+                                            CreationDisposition Disp,
+                                            OpenFlags Flags, unsigned Mode) {
+  return openFile(Name, Disp, FA_Write | FA_Read, Flags);
 }
 
 void closeFile(file_t &F) {
@@ -1239,7 +1277,8 @@ std::error_code real_path(const Twine &p
     return directoryRealPath(path, dest);
 
   int fd;
-  if (std::error_code EC = llvm::sys::fs::openFileForRead(path, fd, &dest))
+  if (std::error_code EC =
+          llvm::sys::fs::openFileForRead(path, fd, OF_None, &dest))
     return EC;
   ::close(fd);
   return std::error_code();

Modified: llvm/trunk/lib/Support/Windows/Program.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Program.inc (original)
+++ llvm/trunk/lib/Support/Windows/Program.inc Thu Jun  7 12:58:58 2018
@@ -496,7 +496,7 @@ std::error_code
 llvm::sys::writeFileWithEncoding(StringRef FileName, StringRef Contents,
                                  WindowsEncodingMethod Encoding) {
   std::error_code EC;
-  llvm::raw_fd_ostream OS(FileName, EC, llvm::sys::fs::OpenFlags::F_Text);
+  llvm::raw_fd_ostream OS(FileName, EC, llvm::sys::fs::F_Text);
   if (EC)
     return EC;
 

Modified: llvm/trunk/lib/Support/raw_ostream.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/lib/Support/raw_ostream.cpp (original)
+++ llvm/trunk/lib/Support/raw_ostream.cpp Thu Jun  7 12:58:58 2018
@@ -498,29 +498,56 @@ void format_object_base::home() {
 //===----------------------------------------------------------------------===//
 
 static int getFD(StringRef Filename, std::error_code &EC,
+                 sys::fs::CreationDisposition Disp, sys::fs::FileAccess Access,
                  sys::fs::OpenFlags Flags) {
+  assert((Access & sys::fs::FA_Write) &&
+         "Cannot make a raw_ostream from a read-only descriptor!");
+
   // Handle "-" as stdout. Note that when we do this, we consider ourself
   // the owner of stdout and may set the "binary" flag globally based on Flags.
   if (Filename == "-") {
     EC = std::error_code();
     // If user requested binary then put stdout into binary mode if
     // possible.
-    if (!(Flags & sys::fs::F_Text))
+    if (!(Flags & sys::fs::OF_Text))
       sys::ChangeStdoutToBinary();
     return STDOUT_FILENO;
   }
 
   int FD;
-  EC = sys::fs::openFileForWrite(Filename, FD, Flags);
+  if (Access & sys::fs::FA_Read)
+    EC = sys::fs::openFileForReadWrite(Filename, FD, Disp, Flags);
+  else
+    EC = sys::fs::openFileForWrite(Filename, FD, Disp, Flags);
   if (EC)
     return -1;
 
   return FD;
 }
 
+raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC)
+    : raw_fd_ostream(Filename, EC, sys::fs::CD_CreateAlways, sys::fs::FA_Write,
+                     sys::fs::OF_None) {}
+
+raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                               sys::fs::CreationDisposition Disp)
+    : raw_fd_ostream(Filename, EC, Disp, sys::fs::FA_Write, sys::fs::OF_None) {}
+
+raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                               sys::fs::FileAccess Access)
+    : raw_fd_ostream(Filename, EC, sys::fs::CD_CreateAlways, Access,
+                     sys::fs::OF_None) {}
+
+raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                               sys::fs::OpenFlags Flags)
+    : raw_fd_ostream(Filename, EC, sys::fs::CD_CreateAlways, sys::fs::FA_Write,
+                     Flags) {}
+
 raw_fd_ostream::raw_fd_ostream(StringRef Filename, std::error_code &EC,
+                               sys::fs::CreationDisposition Disp,
+                               sys::fs::FileAccess Access,
                                sys::fs::OpenFlags Flags)
-    : raw_fd_ostream(getFD(Filename, EC, Flags), true) {}
+    : raw_fd_ostream(getFD(Filename, EC, Disp, Access, Flags), true) {}
 
 /// FD is the file descriptor that this writes to.  If ShouldClose is true, this
 /// closes the file when the stream is destroyed.

Modified: llvm/trunk/tools/llvm-ar/llvm-ar.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Thu Jun  7 12:58:58 2018
@@ -390,6 +390,7 @@ static void doExtract(StringRef Name, co
 
   int FD;
   failIfError(sys::fs::openFileForWrite(sys::path::filename(Name), FD,
+                                        sys::fs::CD_CreateAlways,
                                         sys::fs::F_None, Mode),
               Name);
 

Modified: llvm/trunk/tools/llvm-cov/SourceCoverageView.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cov/SourceCoverageView.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cov/SourceCoverageView.cpp (original)
+++ llvm/trunk/tools/llvm-cov/SourceCoverageView.cpp Thu Jun  7 12:58:58 2018
@@ -65,7 +65,8 @@ CoveragePrinter::createOutputStream(Stri
     return errorCodeToError(E);
 
   std::error_code E;
-  raw_ostream *RawStream = new raw_fd_ostream(FullPath, E, sys::fs::F_RW);
+  raw_ostream *RawStream =
+      new raw_fd_ostream(FullPath, E, sys::fs::FA_Read | sys::fs::FA_Write);
   auto OS = CoveragePrinter::OwnedStream(RawStream);
   if (E)
     return errorCodeToError(E);

Modified: llvm/trunk/tools/llvm-cov/TestingSupport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cov/TestingSupport.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-cov/TestingSupport.cpp (original)
+++ llvm/trunk/tools/llvm-cov/TestingSupport.cpp Thu Jun  7 12:58:58 2018
@@ -75,8 +75,7 @@ int convertForTestingMain(int argc, cons
     return 1;
 
   int FD;
-  if (auto Err =
-          sys::fs::openFileForWrite(OutputFilename, FD, sys::fs::F_None)) {
+  if (auto Err = sys::fs::openFileForWrite(OutputFilename, FD)) {
     errs() << "error: " << Err.message() << "\n";
     return 1;
   }

Modified: llvm/trunk/tools/llvm-exegesis/lib/BenchmarkResult.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-exegesis/lib/BenchmarkResult.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-exegesis/lib/BenchmarkResult.cpp (original)
+++ llvm/trunk/tools/llvm-exegesis/lib/BenchmarkResult.cpp Thu Jun  7 12:58:58 2018
@@ -258,7 +258,8 @@ InstructionBenchmark::writeYaml(const Be
   } else {
     int ResultFD = 0;
     if (auto E = llvm::errorCodeToError(
-            openFileForWrite(Filename, ResultFD, llvm::sys::fs::F_Text))) {
+            openFileForWrite(Filename, ResultFD, llvm::sys::fs::CD_CreateAlways,
+                             llvm::sys::fs::F_Text))) {
       return E;
     }
     llvm::raw_fd_ostream Ostr(ResultFD, true /*shouldClose*/);

Modified: llvm/trunk/tools/llvm-exegesis/llvm-exegesis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-exegesis/llvm-exegesis.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-exegesis/llvm-exegesis.cpp (original)
+++ llvm/trunk/tools/llvm-exegesis/llvm-exegesis.cpp Thu Jun  7 12:58:58 2018
@@ -166,9 +166,12 @@ static void maybeRunAnalysis(const Analy
   }
   std::error_code ErrorCode;
   llvm::raw_fd_ostream ClustersOS(OutputFilename, ErrorCode,
-                                  llvm::sys::fs::F_RW);
-  ExitOnErr(llvm::errorCodeToError(ErrorCode));
-  ExitOnErr(Analyzer.run<Pass>(ClustersOS));
+                                  llvm::sys::fs::FA_Read |
+                                      llvm::sys::fs::FA_Write);
+  if (ErrorCode)
+    llvm::report_fatal_error("cannot open out file: " + OutputFilename);
+  if (auto Err = Analyzer.run<Pass>(ClustersOS))
+    llvm::report_fatal_error(std::move(Err));
 }
 
 static void analysisMain() {

Modified: llvm/trunk/tools/llvm-rc/llvm-rc.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-rc/llvm-rc.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-rc/llvm-rc.cpp (original)
+++ llvm/trunk/tools/llvm-rc/llvm-rc.cpp Thu Jun  7 12:58:58 2018
@@ -171,8 +171,8 @@ int main(int Argc, const char **Argv) {
           "No more than one output file should be provided (using /FO flag).");
 
     std::error_code EC;
-    auto FOut =
-        llvm::make_unique<raw_fd_ostream>(OutArgsInfo[0], EC, sys::fs::F_RW);
+    auto FOut = llvm::make_unique<raw_fd_ostream>(
+        OutArgsInfo[0], EC, sys::fs::FA_Read | sys::fs::FA_Write);
     if (EC)
       fatalError("Error opening output file '" + OutArgsInfo[0] +
                  "': " + EC.message());

Modified: llvm/trunk/unittests/Support/LockFileManagerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/LockFileManagerTest.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/LockFileManagerTest.cpp (original)
+++ llvm/trunk/unittests/Support/LockFileManagerTest.cpp Thu Jun  7 12:58:58 2018
@@ -60,7 +60,7 @@ TEST(LockFileManagerTest, LinkLockExists
   sys::path::append(TmpFileLock, "file.lock-000");
 
   int FD;
-  EC = sys::fs::openFileForWrite(StringRef(TmpFileLock), FD, sys::fs::F_None);
+  EC = sys::fs::openFileForWrite(StringRef(TmpFileLock), FD);
   ASSERT_FALSE(EC);
 
   int Ret = close(FD);

Modified: llvm/trunk/unittests/Support/Path.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/Path.cpp (original)
+++ llvm/trunk/unittests/Support/Path.cpp Thu Jun  7 12:58:58 2018
@@ -49,8 +49,22 @@ using namespace llvm::sys;
   } else {                                                                     \
   }
 
+#define ASSERT_ERROR(x)                                                        \
+  if (!x) {                                                                    \
+    SmallString<128> MessageStorage;                                           \
+    raw_svector_ostream Message(MessageStorage);                               \
+    Message << #x ": did not return a failure error code.\n";                  \
+    GTEST_FATAL_FAILURE_(MessageStorage.c_str());                              \
+  }
+
 namespace {
 
+struct FileDescriptorCloser {
+  explicit FileDescriptorCloser(int FD) : FD(FD) {}
+  ~FileDescriptorCloser() { ::close(FD); }
+  int FD;
+};
+
 TEST(is_separator, Works) {
   EXPECT_TRUE(path::is_separator('/'));
   EXPECT_FALSE(path::is_separator('\0'));
@@ -436,6 +450,7 @@ protected:
   /// Unique temporary directory in which all created filesystem entities must
   /// be placed. It is removed at the end of each test (must be empty).
   SmallString<128> TestDirectory;
+  SmallString<128> NonExistantFile;
 
   void SetUp() override {
     ASSERT_NO_ERROR(
@@ -443,6 +458,11 @@ protected:
     // We don't care about this specific file.
     errs() << "Test Directory: " << TestDirectory << '\n';
     errs().flush();
+    NonExistantFile = TestDirectory;
+
+    // Even though this value is hardcoded, is a 128-bit GUID, so we should be
+    // guaranteed that this file will never exist.
+    sys::path::append(NonExistantFile, "1B28B495C16344CB9822E588CD4C3EF0");
   }
 
   void TearDown() override { ASSERT_NO_ERROR(fs::remove(TestDirectory.str())); }
@@ -1219,8 +1239,8 @@ TEST_F(FileSystemTest, OpenFileForRead)
   // Open the file for read
   int FileDescriptor2;
   SmallString<64> ResultPath;
-  ASSERT_NO_ERROR(
-      fs::openFileForRead(Twine(TempPath), FileDescriptor2, &ResultPath))
+  ASSERT_NO_ERROR(fs::openFileForRead(Twine(TempPath), FileDescriptor2,
+                                      fs::OF_None, &ResultPath))
 
   // If we succeeded, check that the paths are the same (modulo case):
   if (!ResultPath.empty()) {
@@ -1235,6 +1255,209 @@ TEST_F(FileSystemTest, OpenFileForRead)
   ::close(FileDescriptor);
 }
 
+static void createFileWithData(const Twine &Path, bool ShouldExistBefore,
+                               fs::CreationDisposition Disp, StringRef Data) {
+  int FD;
+  ASSERT_EQ(ShouldExistBefore, fs::exists(Path));
+  ASSERT_NO_ERROR(fs::openFileForWrite(Path, FD, Disp));
+  FileDescriptorCloser Closer(FD);
+  ASSERT_TRUE(fs::exists(Path));
+
+  ASSERT_EQ(Data.size(), (size_t)write(FD, Data.data(), Data.size()));
+}
+
+static void verifyFileContents(const Twine &Path, StringRef Contents) {
+  auto Buffer = MemoryBuffer::getFile(Path);
+  ASSERT_TRUE((bool)Buffer);
+  StringRef Data = Buffer.get()->getBuffer();
+  ASSERT_EQ(Data, Contents);
+}
+
+TEST_F(FileSystemTest, CreateNew) {
+  int FD;
+  Optional<FileDescriptorCloser> Closer;
+
+  // Succeeds if the file does not exist.
+  ASSERT_FALSE(fs::exists(NonExistantFile));
+  ASSERT_NO_ERROR(fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateNew));
+  ASSERT_TRUE(fs::exists(NonExistantFile));
+
+  FileRemover Cleanup(NonExistantFile);
+  Closer.emplace(FD);
+
+  // And creates a file of size 0.
+  sys::fs::file_status Status;
+  ASSERT_NO_ERROR(sys::fs::status(FD, Status));
+  EXPECT_EQ(0ULL, Status.getSize());
+
+  // Close this first, before trying to re-open the file.
+  Closer.reset();
+
+  // But fails if the file does exist.
+  ASSERT_ERROR(fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateNew));
+}
+
+TEST_F(FileSystemTest, CreateAlways) {
+  int FD;
+  Optional<FileDescriptorCloser> Closer;
+
+  // Succeeds if the file does not exist.
+  ASSERT_FALSE(fs::exists(NonExistantFile));
+  ASSERT_NO_ERROR(
+      fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateAlways));
+
+  Closer.emplace(FD);
+
+  ASSERT_TRUE(fs::exists(NonExistantFile));
+
+  FileRemover Cleanup(NonExistantFile);
+
+  // And creates a file of size 0.
+  uint64_t FileSize;
+  ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+  ASSERT_EQ(0ULL, FileSize);
+
+  // If we write some data to it re-create it with CreateAlways, it succeeds and
+  // truncates to 0 bytes.
+  ASSERT_EQ(4, write(FD, "Test", 4));
+
+  Closer.reset();
+
+  ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+  ASSERT_EQ(4ULL, FileSize);
+
+  ASSERT_NO_ERROR(
+      fs::openFileForWrite(NonExistantFile, FD, fs::CD_CreateAlways));
+  Closer.emplace(FD);
+  ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+  ASSERT_EQ(0ULL, FileSize);
+}
+
+TEST_F(FileSystemTest, OpenExisting) {
+  int FD;
+
+  // Fails if the file does not exist.
+  ASSERT_FALSE(fs::exists(NonExistantFile));
+  ASSERT_ERROR(fs::openFileForWrite(NonExistantFile, FD, fs::CD_OpenExisting));
+  ASSERT_FALSE(fs::exists(NonExistantFile));
+
+  // Make a dummy file now so that we can try again when the file does exist.
+  createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz");
+  FileRemover Cleanup(NonExistantFile);
+  uint64_t FileSize;
+  ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+  ASSERT_EQ(4ULL, FileSize);
+
+  // If we re-create it with different data, it overwrites rather than
+  // appending.
+  createFileWithData(NonExistantFile, true, fs::CD_OpenExisting, "Buzz");
+  verifyFileContents(NonExistantFile, "Buzz");
+}
+
+TEST_F(FileSystemTest, OpenAlways) {
+  // Succeeds if the file does not exist.
+  createFileWithData(NonExistantFile, false, fs::CD_OpenAlways, "Fizz");
+  FileRemover Cleanup(NonExistantFile);
+  uint64_t FileSize;
+  ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+  ASSERT_EQ(4ULL, FileSize);
+
+  // Now re-open it and write again, verifying the contents get over-written.
+  createFileWithData(NonExistantFile, true, fs::CD_OpenAlways, "Bu");
+  verifyFileContents(NonExistantFile, "Buzz");
+}
+
+TEST_F(FileSystemTest, AppendSetsCorrectFileOffset) {
+  fs::CreationDisposition Disps[] = {fs::CD_CreateAlways, fs::CD_OpenAlways,
+                                     fs::CD_OpenExisting};
+
+  // Write some data and re-open it with every possible disposition (this is a
+  // hack that shouldn't work, but is left for compatibility.  F_Append
+  // overrides
+  // the specified disposition.
+  for (fs::CreationDisposition Disp : Disps) {
+    int FD;
+    Optional<FileDescriptorCloser> Closer;
+
+    createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz");
+
+    FileRemover Cleanup(NonExistantFile);
+
+    uint64_t FileSize;
+    ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+    ASSERT_EQ(4ULL, FileSize);
+    ASSERT_NO_ERROR(
+        fs::openFileForWrite(NonExistantFile, FD, Disp, fs::OF_Append));
+    Closer.emplace(FD);
+    ASSERT_NO_ERROR(sys::fs::file_size(NonExistantFile, FileSize));
+    ASSERT_EQ(4ULL, FileSize);
+
+    ASSERT_EQ(4, write(FD, "Buzz", 4));
+    Closer.reset();
+
+    verifyFileContents(NonExistantFile, "FizzBuzz");
+  }
+}
+
+static void verifyRead(int FD, StringRef Data, bool ShouldSucceed) {
+  std::vector<char> Buffer;
+  Buffer.resize(Data.size());
+  int Result = ::read(FD, Buffer.data(), Buffer.size());
+  if (ShouldSucceed) {
+    ASSERT_EQ((size_t)Result, Data.size());
+    ASSERT_EQ(Data, StringRef(Buffer.data(), Buffer.size()));
+  } else {
+    ASSERT_EQ(-1, Result);
+    ASSERT_EQ(EBADF, errno);
+  }
+}
+
+static void verifyWrite(int FD, StringRef Data, bool ShouldSucceed) {
+  int Result = ::write(FD, Data.data(), Data.size());
+  if (ShouldSucceed)
+    ASSERT_EQ((size_t)Result, Data.size());
+  else {
+    ASSERT_EQ(-1, Result);
+    ASSERT_EQ(EBADF, errno);
+  }
+}
+
+TEST_F(FileSystemTest, ReadOnlyFileCantWrite) {
+  createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz");
+  FileRemover Cleanup(NonExistantFile);
+
+  int FD;
+  ASSERT_NO_ERROR(fs::openFileForRead(NonExistantFile, FD));
+  FileDescriptorCloser Closer(FD);
+
+  verifyWrite(FD, "Buzz", false);
+  verifyRead(FD, "Fizz", true);
+}
+
+TEST_F(FileSystemTest, WriteOnlyFileCantRead) {
+  createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz");
+  FileRemover Cleanup(NonExistantFile);
+
+  int FD;
+  ASSERT_NO_ERROR(
+      fs::openFileForWrite(NonExistantFile, FD, fs::CD_OpenExisting));
+  FileDescriptorCloser Closer(FD);
+  verifyRead(FD, "Fizz", false);
+  verifyWrite(FD, "Buzz", true);
+}
+
+TEST_F(FileSystemTest, ReadWriteFileCanReadOrWrite) {
+  createFileWithData(NonExistantFile, false, fs::CD_CreateNew, "Fizz");
+  FileRemover Cleanup(NonExistantFile);
+
+  int FD;
+  ASSERT_NO_ERROR(fs::openFileForReadWrite(NonExistantFile, FD,
+                                           fs::CD_OpenExisting, fs::OF_None));
+  FileDescriptorCloser Closer(FD);
+  verifyRead(FD, "Fizz", true);
+  verifyWrite(FD, "Buzz", true);
+}
+
 TEST_F(FileSystemTest, set_current_path) {
   SmallString<128> path;
 

Modified: llvm/trunk/unittests/Support/ReplaceFileTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ReplaceFileTest.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/ReplaceFileTest.cpp (original)
+++ llvm/trunk/unittests/Support/ReplaceFileTest.cpp Thu Jun  7 12:58:58 2018
@@ -31,7 +31,7 @@ namespace {
 std::error_code CreateFileWithContent(const SmallString<128> &FilePath,
                                       const StringRef &content) {
   int FD = 0;
-  if (std::error_code ec = fs::openFileForWrite(FilePath, FD, fs::F_None))
+  if (std::error_code ec = fs::openFileForWrite(FilePath, FD))
     return ec;
 
   const bool ShouldClose = true;

Modified: llvm/trunk/unittests/Support/raw_pwrite_stream_test.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/raw_pwrite_stream_test.cpp?rev=334221&r1=334220&r2=334221&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/raw_pwrite_stream_test.cpp (original)
+++ llvm/trunk/unittests/Support/raw_pwrite_stream_test.cpp Thu Jun  7 12:58:58 2018
@@ -84,7 +84,7 @@ TEST(raw_pwrite_ostreamTest, TestFD) {
 #ifdef LLVM_ON_UNIX
 TEST(raw_pwrite_ostreamTest, TestDevNull) {
   int FD;
-  sys::fs::openFileForWrite("/dev/null", FD, sys::fs::F_None);
+  sys::fs::openFileForWrite("/dev/null", FD, sys::fs::CD_OpenExisting);
   raw_fd_ostream OS(FD, true);
   OS << "abcd";
   StringRef Test = "test";




More information about the llvm-commits mailing list