[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-22 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment. My objection to the original patch was including things like the "" lines as remarks to mark a section. I don't think these should be emitted as remarks and are a display function the frontend should take care of if we want any kind of grouping Repository: rG LLV

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D127923#3600422 , @vangthao wrote: > In D127923#3599451 , @aaron.ballman > wrote: > >> Is there a reason the remarks can't use individual diagnostic emissions to >> simulate new

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-21 Thread Vang Thao via Phabricator via cfe-commits
vangthao added a comment. In D127923#3599451 , @aaron.ballman wrote: > Is there a reason the remarks can't use individual diagnostic emissions to > simulate newlines? Or is this perhaps a demonstration that the remarks should > not be using the diagnos

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a subscriber: beanz. aaron.ballman added a comment. I'm a bit uncomfortable with this as we (at least currently) want to discourage non-terse diagnostics, and allowing newlines encourages longer diagnostics. There's an RFC kicking around about making more expressive diagnosti

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-18 Thread Balázs Benics via Phabricator via cfe-commits
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

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-17 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:99-103 + size_t NewlinePos = PD->getShortDescription().find_first_of('\n'); + if (NewlinePos != std::string::npos) +OutStr = PD->getShortDescription().str().insert(

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-17 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment. Thank you for the patch! I will leave it to others with more knowledge of the history of diagnostics to comment on whether the premise is acceptable, but I have a few technical comments Comment at: clang/lib/Frontend/TextDiagnosticPrinter.cpp:119

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-16 Thread Vang Thao via Phabricator via cfe-commits
vangthao updated this revision to Diff 437593. vangthao added a comment. Update commit message to include the motivation behind the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127923/new/ https://reviews.llvm.org/D127923 Files: clang/li

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-16 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Please, consider updating the summary to clearly specify the motivation for making this change. It describes what this patch intends to do, but I'm about the why. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127923/new/

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-16 Thread Gabor Marton via Phabricator via cfe-commits
martong added subscribers: steakhal, NoQ. martong added a comment. @NoQ @steakhal FYI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127923/new/ https://reviews.llvm.org/D127923 ___ cfe-commits mailing li

[PATCH] D127923: [Diagnostics] Accept newline and format diag opts on first line

2022-06-15 Thread Vang Thao via Phabricator via cfe-commits
vangthao created this revision. Herald added a subscriber: martong. Herald added a project: All. vangthao requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Accept newlines when the diagnostic string is being given by an outside source instead