[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb abandoned this revision. cjdb added a comment. Herald added a subscriber: PiotrZSL. Herald added a project: clang-format. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. Abandoning since we're heading down a different design path now. Repository: rG LLVM

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added inline comments. Comment at: clang/test/Frontend/sarif-reason.cpp:15 +void g() { + f1<0>(); // expected-error{{no matching function for call to 'f1'}} + f1(); // expected-error{{no matching function for call to 'f1'}} cjdb wrote: > erichkeane

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. In D138939#3967397 , @tschuett wrote: > I do not ask you to do anything! I just noticed that you add a lot of > `FormatXXXDiagnostic` functions. An alternativ design is to have one > `FormatDiagnostic` function with a mode

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

dblaikie added a comment. Sorry if this is derailing, but I wonder/worry about a few things here: 1. Compounding structured output with phraseology - it seems like it might be worthwhile for these to be separate issues (doesn't mean the terminal output has to say exactly the same thing - as

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. I do not ask you to do anything! I just noticed that you add a lot of `FormatXXXDiagnostic` functions. An alternativ design is to have one `FormatDiagnostic` function with a mode parameter. Then you can decide whether to print legacy or user-oriented reasons. If next

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. In D138939#3966938 , @tschuett wrote: > Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the > output device (console/sarif) are orthogonal issues. > > Still it would nice to be able to toggle the diagnostic

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe the kind/amount of information printed ( `DiagnosticMode` ) and the output device (console/sarif) are orthogonal issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. In D138939#3965985 , @tschuett wrote: > Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing: > > enum class DiagnosticMode { > Legacy, > UserOriented, > Default = Legacy > } It took a fair

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Then Sarif was a distraction. Still to reduce boilerplate and for A/B testing: enum class DiagnosticMode { Legacy, UserOriented, Default = Legacy } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. In D138939#3964677 , @erichkeane wrote: > In D138939#3964439 , @erichkeane > wrote: > >> In D138939#3964404 , @cjdb wrote: >> >>> In

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb updated this revision to Diff 479492. cjdb marked 4 inline comments as done. cjdb added a comment. responds to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138939/new/ https://reviews.llvm.org/D138939 Files:

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3964439 , @erichkeane wrote: > In D138939#3964404 , @cjdb wrote: > >> In D138939#3963496 , @erichkeane >> wrote: >> >>> In

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3964404 , @cjdb wrote: > In D138939#3963496 , @erichkeane > wrote: > >> In D138939#3963473 , @tschuett >> wrote: >> >>> Maybe

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb marked 3 inline comments as done. cjdb added a comment. In D138939#3963496 , @erichkeane wrote: > In D138939#3963473 , @tschuett > wrote: > >> Maybe `void FormatDiagnostic(SmallVectorImpl , DiagnosticMode

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In D138939#3963473 , @tschuett wrote: > Maybe `void FormatDiagnostic(SmallVectorImpl , DiagnosticMode > mode)`instead of `void FormatDiagnostic(SmallVectorImpl )`? > To make the transition easer and future proof. I like this

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

tschuett added a comment. Maybe `void FormatDiagnostic(SmallVectorImpl , DiagnosticMode mode)`instead of `void FormatDiagnostic(SmallVectorImpl )`? To make the transition easer and future proof. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. In 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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

rymiel added inline comments. Comment at: clang/include/clang/Frontend/DiagnosticRenderer.h:130 /// \param Message The diagnostic message to emit. + /// \param Reason Supplementary information for the message. /// \param Ranges The underlined ranges for this code snippet.

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. > 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? Repository:

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. > 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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb added a comment. I don't understand why `test_demangle.pass.cpp` was considered too big to upload. Here's the diff: diff --git a/libcxxabi/test/test_demangle.pass.cpp b/libcxxabi/test/test_demangle.pass.cpp index dce8e6c3..9da6fb7d2ad9 100644 ---

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

erichkeane added a comment. Herald added subscribers: Michael137, JDevlieghere. A few comments. I don't mind the approach, 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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

cjdb created this revision. cjdb added reviewers: aaron.ballman, erichkeane, shafik. Herald added subscribers: carlosgalvezp, kadircet, arphaman. Herald added a reviewer: njames93. Herald added projects: Flang, All. cjdb requested review of this revision. Herald added subscribers: cfe-commits,