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

Tue Oct 4 02:02:31 PDT 2022

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 them, potentially due to the signal handler issues Pavel is referring to.

> Yeah, in my mind all the callbacks should be orthogonal. If it wasn't for the layering, I'd make it all the responsibility of the Diagnostics class. If someone has a better idea than callbacks please let me know.

I do like that they keep the Diagnostics class free of details of the others. Which means you just extend the existing tests for those other classes each time you add a dump function to them.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134991/new/

https://reviews.llvm.org/D134991