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

Reply via email to