[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).


More information about the lldb-commits mailing list