[PATCH] D153417: New CharSetConverter wrapper class for ConverterEBCDIC

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 26 12:39:52 PDT 2023


efriedma added a comment.

In D153417#4449812 <https://reviews.llvm.org/D153417#4449812>, @abhina.sreeskantharajan wrote:

> In D153417#4438764 <https://reviews.llvm.org/D153417#4438764>, @efriedma wrote:
>
>> As I mentioned at https://discourse.llvm.org/t/rfc-enabling-fexec-charset-support-to-llvm-and-clang-reposting/71512 , I think SimplifyLibCalls needs to be aware of encodings.  To make that work, this probably needs to be somewhere in llvm/ , not clang/ .
>
> There was previous resistance to putting this in LLVM https://discourse.llvm.org/t/rfc-adding-a-charset-converter-to-the-llvm-support-library/69795/17, I'm not sure if that sentiment has changed. I think it would be best to have some sort of implementation plan to tackle the SimplifyLibCalls issue before shifting this code

I don't think anyone is particularly against the concept of adding a charset conversion API; most of the discussion is around the choice to implement the API as a thin wrapper around POSIX iconv().  And that concern applies equally no matter where the code is located in the source tree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153417



More information about the cfe-commits mailing list