teemperor added inline comments.
================
Comment at: lldb/include/lldb/Target/AbortRecognizer.h:29
+ GetAbortLocation(Process *process_sp);
+ static llvm::Optional<std::tuple<FileSpec, ConstString>>
+ GetAssertLocation(Process *process_sp);
----------------
This function (especially the return types) deserve some documentation.
================
Comment at: lldb/source/Target/AbortRecognizer.cpp:1
+//===-- AbortRecognizer.cpp -------------------------------------*- C++
-*-===//
+//
----------------
Please remove the `-*- C++ -*-` as that's only for header files.
================
Comment at: lldb/source/Target/AbortRecognizer.cpp:85
+ // Fetch most relevant frame
+ for (uint32_t i = 0; i < frames_to_fetch; i++) {
+ prev_frame_sp = thread_sp->GetStackFrameAtIndex(i);
----------------
Could we make this `frame_index` instead of just `I`. I was confused that `I`
isn't just an index into a vector when you later interpret it as a stack frame
index.
================
Comment at: lldb/source/Target/AbortRecognizer.cpp:99
+ if (sym_ctx.module_sp->GetFileSpec().GetFilename() ==
+ module_spec.GetFilename() &&
+ sym_ctx.GetFunctionName() == function_name) {
----------------
You can just do `sym_ctx.module_sp->GetFileSpec().FileEquals(module_spec)`
which also handles case insensitive file systems IIRC.
================
Comment at: lldb/source/Target/AbortRecognizer.cpp:101
+ sym_ctx.GetFunctionName() == function_name) {
+ if (i < frames_to_fetch - 1)
+ m_most_relevant_frame = thread_sp->GetStackFrameAtIndex(i + 1);
----------------
Maybe add a comment why you need to skip one frame.
================
Comment at: lldb/source/Target/AbortRecognizer.cpp:104
+ else
+ m_most_relevant_frame = thread_sp->GetStackFrameAtIndex(i);
+ break;
----------------
Maybe something like this? Just a suggestion though.
```
lang=c++
uint32_t last_frame_index = frames_to_fetch - 1;
m_most_relevant_frame = thread_sp->GetStackFrameAtIndex(std::min(i + 1,
last_frame_index));
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73303/new/
https://reviews.llvm.org/D73303
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
