[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 12 20:04:52 PDT 2021


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/Support/CharSet.h:98
+  /// \param[in] Source source string
+  /// \param[in,out] Result container for converted string
+  /// \return error code in case something went wrong
----------------
The expectations around null termination and shift state should be documented. Note that https://wg21.link/p2029 recommends that numeric escapes do not trigger changes in shift state, so avoiding automatic transitions to the initial shift state at the end of the (potential mid-string) translation is probably the right thing to do. Using a null `StringRef` as the method to cause a return to the initial shift state would be consistent with the `iconv` interface.


================
Comment at: llvm/include/llvm/Support/CharSet.h:107
+  /// \param[in] Source source string
+  /// \param[in,out] Result container for converted string
+  /// \return error code in case something went wrong
----------------
Same comment about null termination and shift state.


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

https://reviews.llvm.org/D88741



More information about the llvm-commits mailing list