[PATCH] D88741: [SystemZ/z/OS] Add utility class for char set conversion.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Support/CharSet.cpp:178
+  size_t Capacity = Result.capacity();
+  Result.resize(Capacity);
+  char *Output = static_cast<char *>(Result.data());
----------------
My first comment regarding `resize_for_overwrite` was for this line.


================
Comment at: llvm/lib/Support/CharSet.cpp:212
+        [](StringRef Source, SmallVectorImpl<char> &Result) {
+          Result.append(Source.begin(), Source.end());
+          return std::error_code();
----------------
This is consistent with `convertWithTable` but not `convertWithIconv` with respect to whether `Result` is being appended to as opposed to it having its contents replaced. All three should be consistent.


================
Comment at: llvm/unittests/Support/CharSetTest.cpp:42
+static const char CyrillicUTF[] = "\xd0\xaf";
+static const char CyrillicE[] = "\x3f";
+
----------------
Comment should indicate that this is the substitution character.


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

https://reviews.llvm.org/D88741