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

[JF Bastien via Phabricator via cfe-commits]

jfb added a subscriber: hwright.
jfb added a comment.

I worry that changing the general `static_assert` printing (adding a colon, and dropping the quotes) will get @hwright's law to drop on us. We can try and see if e.g. users of clang have automated checks for `static_assert` in their CI pipelines or something. I think your new format looks better, but Hyrum is finicky that way... What do others think?

Can you add tests for other special characters, to make sure they're all handled properly? Just copy/paste my twitter shitpost might be sufficient? I think I covered all the corner cases in it, gotta be thorough!



================
Comment at: clang/lib/Basic/Diagnostic.cpp:818
+      llvm::raw_string_ostream stream(number);
+      stream << "<U+" << llvm::format_hex_no_prefix(c, 4, true) << ">";
+      OutStr.append(number.begin(), number.end());
----------------
We don't have a better hex formatter? 😟
Not a big deal, but I'd hoped that ADT had something!


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16361
+      if (AssertMessage) {
+        assert(isa<StringLiteral>(AssertMessage));
+        if (StringLiteral *MsgStr = cast<StringLiteral>(AssertMessage)) {
----------------
`cast` should already handle this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469