[PATCH] D153417: New CharSetConverter wrapper class for ConverterEBCDIC

Tue Jun 27 05:52:36 PDT 2023

abhina.sreeskantharajan added a comment.

> 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.

Sorry, I misremembered where it was. That discussion I mentioned was from the previous patch https://reviews.llvm.org/D88741, not my RFC.

@cor3ntin had the following concerns:

> If we do use iconv though, i would like us to have a better understanding of use cases, The patch currently links iconv to all llvm libraries, which might be overkill if the only project using it is Clang, and I wonder how that affects packaging
> on linux distributions.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153417