[PATCH] Add writeFileWithSystemEncoding to LibLLVMSupport

[Reid Kleckner]

Looks pretty good. Let me know if you need help landing it when you send a revised patch.

================
Comment at: include/llvm/Support/Program.h:142
@@ +141,3 @@
+  /// \returns true if successful, otherwise \p ErrMsg is filled accordingly.
+  bool writeFileWithEncoding(const char *FileName, const char *contents,
+                             std::string *ErrMsg = nullptr,
----------------
The contents parameter should be a StringRef. Most callers will have the length handy.

================
Comment at: lib/Support/Unix/Program.inc:461-465
@@ +460,7 @@
+
+  if (OS.has_error()) {
+    if (ErrMsg)
+      *ErrMsg = "could not write to file";
+    return false;
+  }
+
----------------
How about returning a std::error_code and using std::errc::io_error here and in the Windows implementation?

================
Comment at: lib/Support/Windows/Program.inc:465-468
@@ +464,6 @@
+    if (std::error_code ec = windows::UTF8ToUTF16(contents, ArgsUTF16)) {
+      if (ErrMsg)
+        *ErrMsg = "unable to convert to UTF-16";
+      SetLastError(ec.value());
+      return false;
+    }
----------------
Similarly, we can just propagate ec here if we return std::error_code.

================
Comment at: lib/Support/Windows/Program.inc:482
@@ +481,3 @@
+
+    if (std::error_code ec = windows::UTF8ToUTF16(contents, ArgsUTF16)) {
+      if (ErrMsg)
----------------
ditto

================
Comment at: unittests/Support/ProgramTest.cpp:275
@@ +274,3 @@
+  ::close(fd);
+  ASSERT_NO_ERROR(fs::remove(Twine(file_pathname)));
+  ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
----------------
Hm, looks like we can't convert from SmallString to Twine. Twine is usually an implementation detail. Can you call .str() here instead?

http://reviews.llvm.org/D4896