[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

efriedma added a comment. If you're having trouble making Arcanist work correctly, you can always just upload "git diff" or "git show" output at https://reviews.llvm.org/differential/diff/create/ . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. There is only one function in ConvertUTFWrapper.cpp: convertUTF32ToUTF8String idk wtf is going on, maybe the ammending the commit is breaking something? the diff I see here is correct... Maybe I should just make a new diff here entirely? CHANGES SINCE LAST ACT

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

efriedma added a comment. As far as I can tell, the lastest version of the diff you uploaded still has the following issues that haven't been addressed: 1. The BOM handling is actually actively a problem if you're planning to use the interface to interpret wprintf format strings. We don't want

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 363216. MarcusJohnson91 added a comment. It seems like this diff keeps getting reverted? I've fixed all the issues mentioned, and the tests work now, everything is formatted correctly too. I've set git up to do full context diffs, but it's not worki

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

efriedma added inline comments. Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:176 + // enough that we can fit a null terminator without reallocating. + Out.resize(SrcBytes.size() * UNI_MAX_UTF8_BYTES_PER_CODE_POINT + 1); + UTF8 *Dst = reinterpret_cast(&Out[0]); --

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362923. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp llvm/unittests/Support/ConvertUTFTest.cpp Index: ll

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362907. MarcusJohnson91 added a comment. Formatted the diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362882. MarcusJohnson91 added a comment. The tests work on my machine now, turns out the Big endian one needs a BOM, pretty obvious in hindsight. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Fi

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. The problem seems to be in the conversion function expecting strings to be a multiple of 4 bytes, which doesn't hold up with the way ArrayRef stores things as char that is casted to char32_t, when using ASCII values like in the look of disapproval emoji, having

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362511. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp llvm/unittests/Support/ConvertUTFTest.cpp Index: ll

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362431. MarcusJohnson91 added a comment. Updated the tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added inline comments. Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:176 + // enough that we can fit a null terminator without reallocating. + Out.resize(SrcBytes.size() * UNI_MAX_UTF8_BYTES_PER_CODE_POINT + 1); + UTF8 *Dst = reinterpret_cast(&Out[0]); ---

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

efriedma added inline comments. Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:176 + // enough that we can fit a null terminator without reallocating. + Out.resize(SrcBytes.size() * UNI_MAX_UTF8_BYTES_PER_CODE_POINT + 1); + UTF8 *Dst = reinterpret_cast(&Out[0]); --

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 362198. MarcusJohnson91 added a comment. Dropped the UTF32 BOM stuff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWra

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

efriedma added a comment. The buildbot seems to think your new unittests are broken. Not sure why. (You can run just the unittests with "ninja check-llvm-unit".) Comment at: llvm/include/llvm/Support/ConvertUTF.h:127 +#define UNI_UTF32_BYTE_ORDER_MARK_NATIVE 0xFEFF +#def

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. Anyone got any ideas what happened this time? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 361535. MarcusJohnson91 marked an inline comment as done. MarcusJohnson91 added a comment. Implemented the fixes mentioned and reformatted the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 marked 3 inline comments as done. MarcusJohnson91 added inline comments. Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:168 + std::vector ByteSwapped; + if (Src[0] == UNI_UTF16_BYTE_ORDER_MARK_SWAPPED) { +ByteSwapped.insert(ByteSwapped.end(), Src, SrcEnd

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

efriedma added a comment. Please fix clang-format issues. (You can use the clang-format-diff.py script.) Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:154 + // Error out on an uneven byte count. + if (SrcBytes.size() % 2) +return false; Wrong consta

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 added a comment. I don't understand why the build failed? I've compiled it and ran all the tests with `time ninja check` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 ___ cfe-commi

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 361487. MarcusJohnson91 edited the summary of this revision. MarcusJohnson91 added a comment. Added tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertU

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 updated this revision to Diff 361485. MarcusJohnson91 added a comment. Added tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106753/new/ https://reviews.llvm.org/D106753 Files: llvm/include/llvm/Support/ConvertUTF.h llvm/lib/Support/ConvertUTFWrapper.cpp llvm/

[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

MarcusJohnson91 created this revision. MarcusJohnson91 added a project: clang. Herald added subscribers: dexonsmith, hiraditya. MarcusJohnson91 requested review of this revision. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Split from D103426