[PATCH] D39922: Create a TempFile class

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 10 14:46:40 PST 2017


rnk added inline comments.


================
Comment at: include/llvm/Support/FileSystem.h:698
 
+class TempFile {
+  bool Done = false;
----------------
Some basic Doxygen here would be good.


================
Comment at: include/llvm/Support/FileSystem.h:703
+public:
+  static Expected<TempFile> create(const Twine &Model,
+                                   unsigned Mode = all_read | all_write);
----------------
Basic doxygen, probably just referring the reader to createUniqueFile.


================
Comment at: include/llvm/Support/FileSystem.h:711
+  // The open file descriptor.
+  int FD;
+
----------------
`= -1`, just in case we forget to initialize FD one day.


================
Comment at: lib/Support/Path.cpp:770
+
+TempFile::~TempFile() { assert(Done); }
+
----------------
Should this attempt to discard the file if it wasn't committed? Otherwise users may need to invent RAII things like:
  DiscardingTempFile : TempFile {
    ~DiscardingTempFile() { if (!Done) consumeError(discard()); }
  };

I guess if we leave it this way, users are discouraged from doing that.


https://reviews.llvm.org/D39922





More information about the llvm-commits mailing list