erichkeane added inline comments.
================
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();
----------------
aaron.ballman wrote:
> 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).
I might prefer `BeginItr`, but I don't see us needing `Begin` for other things
later, so I don't care much.
================
Comment at: clang/lib/Basic/Diagnostic.cpp:807
+static void pushEscapedString(StringRef Str, SmallVectorImpl<char> &OutStr) {
+ OutStr.reserve(OutStr.size() + Str.size());
----------------
could we have some comment as to what this is for/what it is supposed to do?
It isn't clear to me what the arguments mean here. The name 'Str' isn't
particularly helpful.
================
Comment at: clang/lib/Basic/Diagnostic.cpp:843
+ }
+ OutStr.append(Number.begin(), Number.end());
+ continue;
----------------
I find myself wondering if OutStr here should be just a stream as passed in (or
immediately become one above?).
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16592
+ const auto *MsgStr = cast<StringLiteral>(AssertMessage);
+ if (MsgStr->isAscii() == 1)
+ Msg << MsgStr->getString();
----------------
why `isAscii`, which returns bool, compared to 1?
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
