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

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 19 20:39:34 PDT 2021


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Support/CharSet.cpp:104
+        if (Ch != 0xc2 && Ch != 0xc3)
+          return std::make_error_code(std::errc::illegal_byte_sequence);
+        // Is buffer truncated?
----------------
Kai wrote:
> hubert.reinterpretcast wrote:
> > The API contract between the table conversion and the `iconv` conversion is inconsistent. `iconv` conversion performs an implementation-defined conversion for valid input characters that does not have a representation in the output codeset. This implementation fails the conversion instead.
> Changed implementation to have same behavior as iconv.
Thanks for making the change. Thinking about it a bit more, I am wondering if there should be a "policy" available for the class to support selecting one behaviour or the other (or at least identifying that there were characters with no output codeset representation encountered). For example, `iconv` conversions that would not round-trip are identifiable via non-error non-zero return codes (indicating the number of input characters that did not have a representation in the output codeset).

This information/policy will be useful for implementing the diagnostics required if https://wg21.link/p1854 is adopted.


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

https://reviews.llvm.org/D88741



More information about the llvm-commits mailing list