tbaeder added inline comments.
================ 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); + } ---------------- cor3ntin wrote: > tbaeder wrote: > > cor3ntin wrote: > > > 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 > > This is not true in my testing fwiw. > ``` > const unsigned char *Begin = SourceLine.bytes_begin() + *I; > > // Fast path for the common ASCII case. > if (*Begin < 0x80 && llvm::sys::locale::isPrint(*Begin)) { > ++(*I); > return std::make_pair(SmallString<16>(Begin, Begin + 1), true); > } > ``` > seems to work fine locally. Note that I'm not sure `*Begin` is always valid - > it seems to be, but we might want an assert to check that SourceLine is not > empty. This function is only ever called in a `while (I < SourceLine.size())` loop. I've thought about refactoring this into a helper struct that keeps the index separate from the calling function to simplify callers. 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