[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