aaron.ballman added inline comments.
================ Comment at: clang/lib/AST/OSLog.cpp:212 + } else if (Lit->isUTF16()) { + std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t> Convert; + std::u16string U16 = Lit->getStringAsChar16(); ---------------- MarcusJohnson91 wrote: > aaron.ballman wrote: > > cor3ntin wrote: > > > I'm not sure I have a better suggestion but `codecvt_utf8_utf16` is > > > deprecated in C++17 > > Good point -- this likely should be lifted into a common interface (as it > > appears several times in the patch) and make use of the existing LLVM UTF > > conversion functionality: > > https://github.com/intel/llvm/blob/sycl/llvm/include/llvm/Support/ConvertUTF.h > The ConvertUTF version you linked contains `convertUTF32toUTF8String` but the > one in LLVM does not, what's the process for updating this? Oh, oops, I managed to grab a link from downstream and not LLVM, sorry about that! But the downstream and LLVM both have a method to perform the conversion https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/ConvertUTF.h#L162, so you could add convenience functions to that file that wraps the API, as part of this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103426/new/ https://reviews.llvm.org/D103426 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits