steakhal added a comment.
Thanks for updating the summary.
I'm not too confident around diagnostics though.
I'm going to piggyback on @scott.linder's comments. I think they are on the
point.
nit: I'm not sure if `size_t` is a thing in c++, `std::size_t` is though.
================
Comment at: clang/lib/Basic/Diagnostic.cpp:815
for (char c : S) {
- if (llvm::sys::locale::isPrint(c) || c == '\t') {
+ if (llvm::sys::locale::isPrint(c) || c == '\t' || c == '\n') {
OutStr.push_back(c);
----------------
Should we account for `'\r'`-s as well? I'm not sure how it works though
================
Comment at: clang/lib/Frontend/TextDiagnosticPrinter.cpp:119-135
+ // If the diagnostic message is formatted into multiple lines, split the
+ // first line and feed it into printDiagnosticOptions so that we print the
+ // options only on the first line instead of the last line.
+ SmallString<100> InitStr;
+ size_t NewlinePos = OutStr.find_first_of('\n');
+ if (NewlinePos != StringRef::npos) {
+ for (size_t I = 0; I != NewlinePos; ++I)
----------------
scott.linder wrote:
> Nit: I'd remove the `if`s and just use the behavior of `slice(npos, ...)`
> returning the empty string
>
> I also would reword "split the first line and feed it into
> printDiagnosticOptions", as it could be interpreted as meaning the first line
> is an input to `printDiagnosticOptions`
Use braces for both branches or no braces at all. [[
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
| LLVM CodingStandards ]]
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127923/new/
https://reviews.llvm.org/D127923
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
