[PATCH] D108469: Improve handling of static assert messages.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16592
+        StringLiteral *MsgStr = cast<StringLiteral>(AssertMessage);
+        if (MsgStr->getCharByteWidth() == 1)
+          Msg << MsgStr->getString();
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108469/new/

https://reviews.llvm.org/D108469