[PATCH] D47852: [Support] Re-work the Flags parameter of the FileSystem open APIs

[Zachary Turner via Phabricator via llvm-commits]

zturner created this revision.
zturner added a reviewer: rnk.
Herald added subscribers: jkorous, hiraditya, ioeric.

The original motivation for this came from the desire to add a couple more different OpenFlags.  It became clear that such a monolithic and overloaded enumeration was a source of complexity and subtle gotchas.  For example, certain flags could not be specified together, or would be ignored in some situations but not other.  It also became difficult to reason about the subtle differences in each combination of flags.  Finally, I wanted some types of flags to be available even on read operations, but only a couple of the flags actually made sense on that API, while the rest were mostly for writing.

To address this, I split the enumeration into three enumerations.  These are:

`CreationDisposition` - Controls the behavior of the API based on whether or not the file you are trying to open already exists.  It can fail if the file exists, fail if it doesn't exist, truncate, etc.  This takes the place of various uses of `O_Excl`, but also opens up new possibilities.  For example, previously we always specified `O_CREAT`, which means we had no way to force a file to already exist when opening it.

`FileAccess` - Controls whether or not to open the file for read or write.  While this enumeration isn't used in any publicly exported function, I plan to do this in a followup.  Currently it is used interally as a way to re-use code in the private implementation, with all calls to all `openFileForXXX` functions delegating to a single master function.  However, I hope to expose this master function in a follow-up patch.

`OpenFlags` - What used to be a large monoloithic enumeration is now just 3 values.  Append, Text, and Delete.  Any combination of these values can be used together, unlike before.

Another advantage of doing things this way is that it becomes very easy to write exhaustive test coverage.  I've provided tests for every new creation disposition as well as the Append open flag, none of which was thoroughly tested before.

Tested on Linux and Windows.


https://reviews.llvm.org/D47852

Files:
  clang-tools-extra/clang-move/tool/ClangMoveMain.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang/lib/Basic/VirtualFileSystem.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lldb/source/Core/Debugger.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/raw_ostream.h
  llvm/lib/Support/FileOutputBuffer.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/TarWriter.cpp
  llvm/lib/Support/Unix/Path.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/lib/Support/Windows/Program.inc
  llvm/lib/Support/raw_ostream.cpp
  llvm/tools/llvm-ar/llvm-ar.cpp
  llvm/tools/llvm-cov/SourceCoverageView.cpp
  llvm/tools/llvm-cov/TestingSupport.cpp
  llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
  llvm/tools/llvm-exegesis/llvm-exegesis.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/unittests/Support/LockFileManagerTest.cpp
  llvm/unittests/Support/Path.cpp
  llvm/unittests/Support/ReplaceFileTest.cpp
  llvm/unittests/Support/raw_pwrite_stream_test.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47852.150220.patch
Type: text/x-patch
Size: 51468 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180606/60c9d269/attachment.bin>