aaron.ballman added a comment. Patch application is still failing the precommit CI; it'd be nice to get that corrected for some early test feedback.
================ Comment at: clang/docs/ReleaseNotes.rst:275 This fixes `Issue 55962 <https://github.com/llvm/llvm-project/issues/55962>`_. +- Printable Unicode characters within `static_assert` messages are no longer + escaped. ---------------- ================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16592 + StringLiteral *MsgStr = cast<StringLiteral>(AssertMessage); + if (MsgStr->getCharByteWidth() == 1) + Msg << MsgStr->getString(); ---------------- cor3ntin wrote: > cor3ntin wrote: > > 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. > Done. > with this change > > > ``` > static_assert(false, "Ω"); // outputs Ω > static_assert(false, u8"Ω"); // ouputs u8"\316\251" > ``` > > I think this makes sense. Or rather using `u8` in static)assert makes no > sense so it's not an issue, even a good thing not to have any special > handling. disuade people from using prefixes in that context. >>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. Ah derp, good point. Thanks for the switch to `isASCII()` -- I agree that we shouldn't go to great lengths to support prefixes there until WG21 decides what to do with compile-time strings. 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
