cor3ntin added a comment. This generally looks good to me but i have a couple remarks
================ Comment at: clang/lib/Frontend/TextDiagnostic.cpp:124-128 + if (CharSize == 1 && llvm::isLegalUTF8Sequence(Begin, End) && + llvm::sys::locale::isPrint(*Begin)) { + ++(*I); + return std::make_pair(SmallString<16>(Begin, End), true); + } ---------------- this could be simplified : the common case for ascii could be just looking at `isPrint(*Begin);` (which implies CharSize == 1 and llvm::isLegalUTF8Sequence(Begin, End)) So you could do it before computing CharSize ================ Comment at: clang/lib/Frontend/TextDiagnostic.cpp:132 + // Convert it to UTF32 and check if it's printable. + if (llvm::isLegalUTF8Sequence(Begin, End)) { + llvm::UTF32 C; ---------------- I think we should check that `CharSize` is not greater than `Begin + SourceLine.size() - *i` or something along those lines otherwise we may overflow SourceLine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150843/new/ https://reviews.llvm.org/D150843 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits