[PATCH] D114342: ConvertUTF, new wrapper API

Corentin Jabot via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 21 10:55:31 PDT 2022


cor3ntin accepted this revision.
cor3ntin added a comment.

Beside the missing comment Michael wanted, I think this looks consistent with the existing functions



================
Comment at: llvm/lib/Support/ConvertUTFWrapper.cpp:172
+  // enough that we can fit a null terminator without reallocating.
+  Out.resize(SrcBytes.size() + 1);
+  UTF8 *Dst = reinterpret_cast<UTF8 *>(&Out[0]);
----------------
Bigcheese wrote:
> This is technically correct, but it's implicit in that the max number of UTF8 code units per code point is the same as `sizeof(UTF32)`. Would be nice to have a comment.
Nit: The comment still doesn't say that we assume there can only be 4 bytes per utf-8 code units - which would not be the case if the utf-8 comes for non-iso10646 conforming android environments for example


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

https://reviews.llvm.org/D114342



More information about the llvm-commits mailing list