aaron.ballman added a comment.
In D108469#2957652 <https://reviews.llvm.org/D108469#2957652>, @jfb wrote:
> 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?
I think it's fine to change; we've never promised diagnostic formatting
compatibility between versions. I'm sure *someone* is relying on this
somewhere, but I'm not worried we're going to break a ton of people --
hopefully enough folks are tracking trunk that we can find any major issues
before releasing.
Btw, it looks like the CI is currently failing the LLVM unit tests in
interesting ways. That should be resolved.
There are changes in `clang/test/Lexer/null-character-in-literal.c` but Phab is
unhelpful about showing what those changes are because it thinks the file is a
binary file. Can you explain what's been changed there?
================
Comment at: clang/lib/Basic/Diagnostic.cpp:791
+static void PushEscapedString(StringRef Str, SmallVectorImpl<char> &OutStr) {
+ OutStr.reserve(OutStr.size() + Str.size());
----------------
Per coding conventions.
================
Comment at: clang/lib/Basic/Diagnostic.cpp:793
+ OutStr.reserve(OutStr.size() + Str.size());
+ auto it = reinterpret_cast<const unsigned char *>(Str.data());
+ auto end = it + Str.size();
----------------
Please fix the clang-tidy and clang-format warnings.
Also, I think `it` is more commonly considered the name for an iterator, which
this sort of isn't. I'd recommend going with `Begin` and `End` for names (that
also fixes the coding style nit with the names).
================
Comment at: clang/lib/Basic/Diagnostic.cpp:803-806
+ llvm::UTF32 c;
+ llvm::UTF32 *cptr = &c;
+ unsigned char const *start = it;
+ unsigned char const *cp_end = it + llvm::getNumBytesForUTF8(*it);
----------------
You should fix these names up for the coding conventions (same suggestion
applies elsewhere). Also, the local style is `const unsigned char *`.
================
Comment at: clang/lib/Basic/Diagnostic.cpp:819
+ stream << "<U+" << llvm::format_hex_no_prefix(c, 4, true) << ">";
+ OutStr.append(number.begin(), number.end());
+ continue;
----------------
I'm pretty sure you need to call `flush()` before using `number`.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16361-16363
+ if (StringLiteral *MsgStr = cast<StringLiteral>(AssertMessage)) {
+ Msg << MsgStr->getString();
+ }
----------------
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
