cor3ntin added inline comments.
================ Comment at: clang/lib/Basic/Diagnostic.cpp:821 + llvm::UTF32 CodepointValue; + llvm::UTF32 *CpPtr = &CodepointValue; + const unsigned char *CodepointBegin = Begin; ---------------- aaron.ballman wrote: > We don't really need this variable, we can use `&CodepointValue` in the two > places it's needed. We do need a `llvm::UTF32**` though - that's why there is this otherwise not useful variable ================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16592 + StringLiteral *MsgStr = cast<StringLiteral>(AssertMessage); + if (MsgStr->getCharByteWidth() == 1) + Msg << MsgStr->getString(); ---------------- aaron.ballman wrote: > Do we want to use something like `isASCII()` here instead of just checking > for a single-byte width? e.g., pascal strings are single byte width, but > probably not something we want printed this way. > > Another question is, do we want to do something slightly slower (since we > know we're already on the failure path, the performance here shouldn't matter > overly much) by checking `!containsNonAsciiOrNull()`? > Do we want to use something like isASCII() here instead of just checking for > a single-byte width? e.g., pascal strings are single byte width, but probably > not something we want printed this way. Maybe yes. I though about supporting `u8`, but we should definitively not make effort for string with a prefix > Another question is, do we want to do something slightly slower (since we > know we're already on the failure path, the performance here shouldn't matter > overly much) by checking !containsNonAsciiOrNull() No, we want to render unicode characters properly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
