[Lldb-commits] [PATCH] D49322: Narrow the CompletionRequest API to being append-only.
Raphael Isemann via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Jul 25 14:41:16 PDT 2018
teemperor marked an inline comment as done.
teemperor added inline comments.
================
Comment at: source/Commands/CommandObjectMultiword.cpp:378
return proxy_command->HandleArgumentCompletion(request, opt_element_vector);
- request.GetMatches().Clear();
return 0;
----------------
davide wrote:
> I'm not sure not calling `Clear` here is correct.
Not calling `Clear` could only influence the results if the `Clear` previously deleted existed completions from the list (which from what I can tell never happens and if would be a bug). It's most likely just a failsafe because we return 0 from this function (which has the match the size of match array).
================
Comment at: source/Interpreter/Options.cpp:731-741
- // The options definitions table has duplicates because of the
- // way the grouping information is stored, so only add once.
- bool duplicate = false;
- for (size_t k = 0; k < request.GetMatches().GetSize(); k++) {
- if (request.GetMatches().GetStringAtIndex(k) == full_name) {
- duplicate = true;
- break;
----------------
davide wrote:
> Why did you remove this code?
The `AddCompletion` is already checking for duplicates, so this code becomes redundant. Actually, the only reason `AddCompletion` is checking for duplicates is to make this code redundant (as one can't implement duplicate filtering without read access to the match list, which we removed in this patch).
https://reviews.llvm.org/D49322
More information about the lldb-commits
mailing list