[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