[PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons
Erich Keane via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 1 12:48:43 PST 2022
erichkeane added a comment.
In D138939#3964439 <https://reviews.llvm.org/D138939#3964439>, @erichkeane wrote:
> In D138939#3964404 <https://reviews.llvm.org/D138939#3964404>, @cjdb wrote:
>
>> In D138939#3963496 <https://reviews.llvm.org/D138939#3963496>, @erichkeane wrote:
>>
>>> In D138939#3963473 <https://reviews.llvm.org/D138939#3963473>, @tschuett wrote:
>>>
>>>> Maybe `void FormatDiagnostic(SmallVectorImpl<char> &OutStr, DiagnosticMode mode)`instead of `void FormatDiagnostic(SmallVectorImpl<char> &OutStr)`?
>>>> To make the transition easer and future proof.
>>>
>>> I like this idea. Perhaps with DiagnosticMode being a 3-way enum:
>>>
>>> enum struct DiagnosticMode {
>>> Legacy,
>>> Sarif,
>>> Default = Legacy
>>> }
>>>
>>> I like the idea in particular, since it makes a command line flag to modify "Default" to be whichever the user prefers pretty trivial.
>>
>> There's already a flag for this: `-fdiagnostics-format=sarif`. Why do we need a second diagnostic mode flag?
>
> Ah, oh... is the Sarif formatting being done with a new formatter? That seems unfortunate, since folks using the other formatters won't be able to use the user friendly formats.
I've been alerted offline that I am misunderstanding the Sarif proposal, and where this is going. I'll note that I wasn't present/invited at the calls where all of this was discussed, so I am admittedly not completely up to date. The above concern shouldn't stop others from reviewing this, particularly if you better understand the intent here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138939/new/
https://reviews.llvm.org/D138939
More information about the cfe-commits
mailing list