JDevlieghere added inline comments.
================
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:966
name = "(internal)";
result.GetOutputStream().Printf(
+ "%d: %s, module %s, function %s{%s}%s\n", recognizer_id,
----------------
We should stream to the output stream directly to avoid all these useless
conversions.
================
Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:29
/// Otherwise, returns \a llvm::None.
-llvm::Optional<std::tuple<FileSpec, StringRef>>
+llvm::Optional<std::tuple<FileSpec, StringRef, StringRef>>
GetAbortLocation(Process *process) {
----------------
With two elements in the tuple I was torn between a struct with named fields,
but with 3 fields I strongly believe that would be the better choice.
================
Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:152
+ ConstString func_name = sym_ctx.GetFunctionName();
+ if (func_name == ConstString(function_name) ||
+ alternate_function_name.empty() ||
----------------
I believe someone added an overload for comparing ConstStrings with StringRefs.
We shouldn't have to pay the price of creating one here just for comparison.
Same below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74252/new/
https://reviews.llvm.org/D74252
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
