[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:175-178
+  if (CSFrom == CS_IBM1047)
+    Table = IBM1047ToISO88591;
+  if (CSTo == CS_IBM1047)
+    Table = ISO88591ToIBM1047;
----------------
This is a public constructor. It would be appropriate to check that `CSFrom` and `CSTo` aren't both `CS_IBM1047`...


================
Comment at: llvm/lib/Support/CharSet.cpp:179-183
+  return CharSetConverter{
+      [Table, Flags](StringRef Source, SmallVectorImpl<char> &Result) {
+        return convertWithTable(Table, Flags, Source, Result);
+      },
+      nullptr};
----------------
Because of the [U+0000, U+00FF] limitation of the `convertWithTable` converter, we should not be getting here if there is a no-op conversion from UTF-8 to UTF-8. Either a no-op converter should be returned, or a request for such a converter is erroneous (`report_fatal_error` here would be friendlier by failing fast).


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

https://reviews.llvm.org/D88741