[PATCH] Add writeFileWithSystemEncoding to LibLLVMSupport

[Rafael Ávila de Espíndola]

Almost there. Thanks for testing all the strange codepage combinations!

================
Comment at: include/llvm/Support/Program.h:146
@@ +145,3 @@
+  /// \returns true if successful, otherwise \p ErrMsg is filled accordingly.
+  std::error_code writeFileWithEncoding(const char *FileName, StringRef contents,
+                                        std::string *ErrMsg = nullptr,
----------------
Please don't pass in the ErrMsg string. The caller can use EC.message() method. That is an old API design we try to avoid. I see it is used because raw_fd_osstream was never updated to avoid it :-(

Suggestion: pass in a file descriptor. That way you can use a raw_fd_ostream with a saner interface and we don't spread the use of std::string pointers for error messages to other APIs.

Another option: keep the filename, but use openFileForWrite + the fd raw_fd_ostream constructor.

================
Comment at: lib/Support/Unix/Program.inc:479
@@ -450,3 +478,3 @@
   // Conservatively account for space required by environment variables.
-  ArgMax /= 2;
+  long halfArgMax = ArgMax / 2;
 
----------------
This bug fix could be in another patch, no?

================
Comment at: lib/Support/Windows/Program.inc:171
@@ -168,15 +170,3 @@
 
-static bool Execute(ProcessInfo &PI, StringRef Program, const char **args,
-                    const char **envp, const StringRef **redirects,
-                    unsigned memoryLimit, std::string *ErrMsg) {
-  if (!sys::fs::can_execute(Program)) {
-    if (ErrMsg)
-      *ErrMsg = "program not executable";
-    return false;
-  }
-
-  // Windows wants a command line, not an array of args, to pass to the new
-  // process.  We have to concatenate them all, while quoting the args that
-  // have embedded spaces (or are empty).
-
+static std::unique_ptr<char[]> flattenArgs(const char **args) {
   // First, determine the length of the command line.
----------------
This refactoring could be in another patch, no?

http://reviews.llvm.org/D4896