[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons
Erich Keane via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Dec 1 06:11:39 PST 2022
erichkeane added a comment.
In D138939#3958185 <https://reviews.llvm.org/D138939#3958185>, @cjdb wrote:
>> The clang-side interface to this seems a touch clunky, and I fear it'll make continuing its use/generalizing its use less likely.
>
> Is this w.r.t. the `FormatDiagnostic` being split up, or is it something else? If it's the former: I'll be changing that to `FormatLegacyDiagnostic`, which //almost// gets us back to where we started.
Urgh, I was a bit afraid you'd ask that :D It is more of a feeling I guess, which is perhaps biased by this patch being particularly in the diagnostics-handling code. However, I suspect that over time, you're going to want to start switching all these uses of `FormatLegacyReason` over to `FormatSarifReason`, and I would want the 'easy way' to be the 'right' way in either case. Having it be a 2-liner, or over-specifying what is otherwise the 'default' behavior is a bit disconcerting.
In D138939#3958231 <https://reviews.llvm.org/D138939#3958231>, @cjdb wrote:
>> though I find myself wondering if the "FormatDiagnostic" call should stay the same, and choose the legacy-reason only when a sarif reason doesn't exist? Or for some level of command line control?
>
> Hmm... isn't this the point of the diagnostic consumers?
I don't have a great feeling of that? I haven't done much thinking into the diagnostics architecture over the years. That said, the use of when we'd choose one vs the other isn't completely clear to me yet.
================
Comment at: clang/lib/Basic/Diagnostic.cpp:791
/// FormatDiagnostic - Format this diagnostic into a string, substituting the
/// formal arguments into the %0 slots. The result is appended onto the Str
----------------
Comment no longer matches.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138939/new/
https://reviews.llvm.org/D138939
More information about the lldb-commits
mailing list