[PATCH] Add writeFileWithSystemEncoding to LibLLVMSupport

[Rafael Auler]

Hi Rafael,

Thanks for this extra round of review, I answered your questions below. I will send an updated patch shortly.

================
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,
----------------
rafael wrote:
> 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.
I will use your updated constructor from r216293, thanks for updating it!

================
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;
 
----------------
rafael wrote:
> This bug fix could be in another patch, no?
Ok! Sent it in http://reviews.llvm.org/D5053

================
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.
----------------
rafael wrote:
> This refactoring could be in another patch, no?
Ok! Sent it in http://reviews.llvm.org/D5054

http://reviews.llvm.org/D4896