[PATCH] Add writeFileWithSystemEncoding to LibLLVMSupport

Rafael Auler rafaelauler at gmail.com
Tue Aug 26 12:40:41 PDT 2014


I don't know why, but phabricator didn't send the email here updating this
thread. Anyway, I sent a new patch addressing your concerns. It is
available at http://reviews.llvm.org/D4896.

Description:

 Introduce "WindowsEncodingMethod" to stress that the file encoding is only
relevant on Windows systems. Remove the old "EncodingStrategy", addressing
Rafael's concerns.



On Tue, Aug 26, 2014 at 11:41 AM, Rafael Ávila de Espíndola <
rafael.espindola at gmail.com> wrote:

> ================
> Comment at: include/llvm/Support/Program.h:140
> @@ +139,3 @@
> +  struct EncodingStrategy {
> +    EncodingMethod UnixEncoding;
> +    EncodingMethod WindowsEncoding;
> ----------------
> When is the UnixEncoding not UTF8?
>
> If the problem was just the assert, I would suggest just writing the Unix
> version as
>
>         ​std::error_code llvm::sys::writeFileWithEncoding(const char
> *FileName,
>>  StringRef Contents,
>>  EncodingStrategy /*ignored*/) {
>
> and documenting UTF8 is always used on Unix. You can even name enum
> WindowsEncodingMethod to make it explicit that is why we use on Windows.
>
> ================
> Comment at: include/llvm/Support/Program.h:154
> @@ +153,3 @@
> +  std::error_code
> +  writeFileWithEncoding(const char *FileName, StringRef Contents,
> +                        EncodingStrategy Encoding = {EM_UTF8, EM_UTF8});
> ----------------
> FileName can be a StringRef now, no?
>
> http://reviews.llvm.org/D4896
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140826/de0351f2/attachment.html>


More information about the llvm-commits mailing list