JDevlieghere added a comment.
A few more nits as I was reading through the code, but this generally LGTM.
================
Comment at: lldb/source/Commands/CommandCompletions.cpp:811-813
+ sort(to_delete.begin(), to_delete.end(), std::greater<size_t>());
+ for (size_t idx : to_delete)
+ args.DeleteArgumentAtIndex(idx);
----------------
jingham wrote:
> JDevlieghere wrote:
> > I'm surprised this works. Don't the indexes shift when one's deleted? I
> > assumed that's why you're sorting them.
> Why wouldn't it?
>
> The delete starts with the greatest index, so each time you delete an entry
> it shuffles the ones above it down, but it doesn't reshuffle the ones below
> it. Since all the remaining indices are lower than the one you just deleted,
> they are all still pointing to the elements you intend to remove.
Got it, I got the direction wrong.
================
Comment at: lldb/source/Commands/CommandObjectCommands.cpp:2028
+ if (num_args == 0) {
+ result.AppendError("No command was specified.");
+ return false;
----------------
This method still has some inconsistencies in capitalization.
================
Comment at: lldb/source/Commands/CommandObjectMultiword.cpp:34-35
+
+ CommandObject::CommandMap::iterator pos;
+ pos = m_subcommand_dict.find(std::string(sub_cmd));
+ if (pos == m_subcommand_dict.end())
----------------
================
Comment at: lldb/source/Commands/CommandObjectMultiword.cpp:54-67
CommandObject::CommandMap::iterator pos;
- if (!m_subcommand_dict.empty()) {
+ StringList local_matches;
+ if (matches == nullptr)
+ matches = &local_matches;
+ int num_matches =
+ AddNamesMatchingPartialString(m_subcommand_dict, sub_cmd, *matches);
----------------
================
Comment at: lldb/source/Commands/CommandObjectMultiword.cpp:112-115
+ CommandMap::iterator pos;
+ std::string str_name(name);
+
+ pos = m_subcommand_dict.find(str_name);
----------------
================
Comment at: lldb/source/Commands/CommandObjectMultiword.cpp:121-129
+ const char *error_str = nullptr;
+ if (!can_replace)
+ error_str = "sub-command already exists";
+ if (!(*pos).second->IsUserCommand())
+ error_str = "can't replace a builtin subcommand";
+
+ if (error_str) {
----------------
This inline diff looks more confusing than anything, but basically I just
switched the errors around (instead of the current fall-through) and return the
error immediately.
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:1259-1262
+ StringList *matches_ptr = matches;
+ StringList tmp_list;
+ if (!matches_ptr)
+ matches_ptr = &tmp_list;
----------------
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:1369-1370
+bool CommandInterpreter::RemoveUserMultiword(llvm::StringRef multi_name) {
+ CommandObject::CommandMap::iterator pos =
+ m_user_mw_dict.find(std::string(multi_name));
+ if (pos != m_user_mw_dict.end()) {
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110298/new/
https://reviews.llvm.org/D110298
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
