friss added inline comments.
================
Comment at: lldb/include/lldb/Target/StackFrameRecognizer.h:115
std::function<void(uint32_t recognizer_id, std::string recognizer_name,
- std::string module, std::string symbol,
- std::string alternate_symbol, bool regexp)> const
- &callback);
+ std::string module, std::vector<ConstString> &symbols,
+ bool regexp)> const &callback);
----------------
can this be a const vector, or even better an ArrayRef? We certainly don't want
the callbacks to modify the list?
================
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:749
case 'n':
- m_function = std::string(option_arg);
+ m_symbols.push_back(std::string(option_arg));
break;
----------------
Does the command actually accept multiple symbols names now? If yes, it should
be tested and a warning should be added when you use the regex option and you
pass multiple -n options. If it doesn't accept multiple names, it shouldn't
store a vector.
================
Comment at: lldb/source/Commands/CommandObjectFrame.cpp:859
+ result.AppendErrorWithFormat(
+ "%s needs at lease one symbol name (-n argument).\n",
+ m_cmd_name.c_str());
----------------
*at least
================
Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:145
- if (func_name == location.symbol_name ||
- (!location.alternate_symbol_name.IsEmpty() &&
- func_name == location.alternate_symbol_name)) {
-
+ if (location.HasSymbol(func_name)) {
// We go a frame beyond the assert location because the most relevant
----------------
This reads a little weird to me. Should we call it MatchesSymbol instead?
================
Comment at: lldb/source/Target/StackFrameRecognizer.cpp:64-72
+ m_recognizers.push_front({(uint32_t)m_recognizers.size(),
+ false,
+ recognizer,
+ true,
+ ConstString(),
+ module,
+ {},
----------------
Is this really the clang-format formatting?
================
Comment at: lldb/source/Target/StackFrameRecognizer.cpp:89-90
+ entry.symbols.clear();
+ entry.symbols.push_back(ConstString(symbol_name));
+
----------------
It's kinda weird to do this here, and I also think it's wrong as it will leave
the recognizer with an entry in its symbols list which changes its semantics.
If the callback was taking an ArrayRef, it would be easy (and cheap) to create
one for a single element as well as from a vector.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76188/new/
https://reviews.llvm.org/D76188
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
