[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG84ea6b6f78df: [lldb] Add diagnostics (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Gith

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere updated this revision to Diff 470975. JDevlieghere added a comment. Remove spurious `lldb` subdir CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134991/new/ https://reviews.llvm.org/D134991 Files: lldb/include/lldb/API/SBDebugger.h lldb/include/lldb/Utility/Diagnostics.

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/lldb/include/lldb/Utility/Diagnostics.h:1 +//===-- Diagnostics.h ---*- C++ -*-===// +// --

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134991/new/ https://reviews.llvm.org/D134991 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commi

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere updated this revision to Diff 469399. JDevlieghere marked 2 inline comments as done. JDevlieghere added a comment. Split off the statistics CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134991/new/ https://reviews.llvm.org/D134991 Files: lldb/include/lldb/API/SBDebugger.

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/source/Target/Statistics.cpp:305 +std::error_code ec; +raw_fd_ostream stats_stream(stat_file.GetPath(), ec, sys::fs::OF_None); +if (ec) clayborg wrote:

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

DavidSpickett added inline comments. Comment at: lldb/source/Utility/Diagnostics.cpp:43 + std::lock_guard guard(m_callbacks_mutex); + m_callbacks.push_back(callback); +} DavidSpickett wrote: > Is it worth adding an assert that the callback is not already in the

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

clayborg added inline comments. Comment at: lldb/source/Target/Statistics.cpp:293 + +llvm::Error DebuggerStats::Dump(const FileSpec &dir) { + for (size_t debugger_idx = 0; debugger_idx < Debugger::GetNumDebuggers(); Should this return a Expected> in case the cal

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere updated this revision to Diff 466549. JDevlieghere added a comment. Changed my mind (again). I made the Callback a std::function so we can use lambas. I used function pointers originally so we could easily compare and remove callbacks, but there's too many places where this overcomp

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere updated this revision to Diff 466526. JDevlieghere marked 3 inline comments as done. JDevlieghere added a comment. Address code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134991/new/ https://reviews.llvm.org/D134991 Files: lldb/include/lldb/API/SBDebug

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere marked 4 inline comments as done. JDevlieghere added inline comments. Comment at: lldb/include/lldb/Utility/Log.h:231 llvm::StringRef function, const char *format, - Args &&... args) { + Args &&...args) { F

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

DavidSpickett added a comment. > I think that part should be easy to test and I was planning on adding a test > as part of that. Sounds good to me. Nothing needed in this change then. A command also gives you a chance to get the diagnostics out if running until the actual crash fails to dump t

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere added a comment. In D134991#3830682 , @DavidSpickett wrote: > I like the direction, thanks for working on this. > > To no one's surprise, I was thinking about testing :) > > - Errors related to the filesystem e.g. can't open a path, is too c

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

labath added a subscriber: rupprecht. labath added a comment. Adding @rupprecht, as he might be interested in following this. I don't want to get too involved in this, but the first thought on my mind is "do you really want to do all this from within a signal handler?". The fact that we're runn

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

DavidSpickett added a comment. I like the direction, thanks for working on this. To no one's surprise, I was thinking about testing :) - Errors related to the filesystem e.g. can't open a path, is too complicated to setup on demand. The code looks to be passing on the error, which is good enou

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere updated this revision to Diff 464396. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134991/new/ https://reviews.llvm.org/D134991 Files: lldb/include/lldb/API/SBDebugger.h lldb/include/lldb/Target/Statistics.h lldb/include/lldb/Utility/Diagnostics.h lldb/include/lldb/

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere updated this revision to Diff 464395. JDevlieghere added a comment. Fix baseline CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134991/new/ https://reviews.llvm.org/D134991 Files: lldb/include/lldb/API/SBDebugger.h lldb/include/lldb/Target/Statistics.h lldb/include/ll

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

JDevlieghere created this revision. JDevlieghere added reviewers: labath, clayborg, DavidSpickett, jingham, mib. Herald added a project: All. JDevlieghere requested review of this revision. Around this time last year, I said on the mailing list [1] that I wanted to to transform the reproducers in