[PATCH] D106753: ConvertUTF: Created wrapper convertUTF32ToUTF8String

[Eli Friedman via Phabricator via llvm-commits]

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 0x0000FEFF
+#define UNI_UTF32_BYTE_ORDER_MARK_SWAPPED 0x0000FFFE
+
----------------
UNI_UTF32_BYTE_ORDER_MARK_SWAPPED doesn't have the correct bit pattern.


================
Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:168
+  std::vector<UTF32> ByteSwapped;
+  if (Src[0] == UNI_UTF16_BYTE_ORDER_MARK_SWAPPED) {
+    ByteSwapped.insert(ByteSwapped.end(), Src, SrcEnd);
----------------
MarcusJohnson91 wrote:
> efriedma wrote:
> > Wrong constant.
> > 
> > Is this really the function you want to be using from clang?  I don't really understand why you'd want to handle byte order marks.
> I don't really care about the BOM tbh, I just figured if I was in here, I should flesh out the UTF-32 interface.
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 to byteswap `L"\uFFFE%s"` or something like that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106753/new/

https://reviews.llvm.org/D106753