<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">I have been looking at the same issue (duplicate completions), and it looks like the root issue is that CommandInterpreter::GetCommandSP() is acting both as an exact matcher (returning a CommandObjectSP) and as an inexact matcher (filling up a StringList), and is hardly effective at this dual task (its return value only says whether the exact match worked, not whether the inexact match also worked)<div>I am thinking that a good solution might be:</div><div> - try to get an exact match and do not fill the list</div><div>- if you get an exact match, then add that command to the list and bail out</div><div>- if you did not get an exact match, only then try to get inexact matches into the StringList</div><div><br></div><div>Another alternative would, of course, be to have a InsertUnique() in StringList, even though either the RemoveDups() and the InsertUnique() seem more hacks around the issue than anything, at least IMHO.<br><div><div><div><br><div>
<div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style=" orphans: 2; widows: 2; border-collapse: separate; border-spacing: 0px;"><span style="font-size: 12px; orphans: auto; widows: auto;">Enrico Granata</span><br style="font-size: 12px; orphans: auto; widows: auto;"><span style="font-size: 12px; orphans: auto; widows: auto;">📩 egranata@</span><font color="#ff2600" style="font-size: 12px; orphans: auto; widows: auto;"></font><span style="font-size: 12px; orphans: auto; widows: auto;">.com</span><br style="font-size: 12px; orphans: auto; widows: auto;"><span style="font-size: 12px; orphans: auto; widows: auto;">☎️ 27683</span></div></div></div>
</div>
<br><div><div>On Jun 17, 2013, at 4:32 PM, Matthew Sorrels <<a href="mailto:sorrels.m@gmail.com">sorrels.m@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr"><div>Thanks for fixing that. I thought I was losing my mind there for a second. There are still issues with running the tests on Linux though (you need to use clang and you must add a -lstdc++ to the test/make/Makefile.rules to just get the number of failures down to only 4). And so many tests are skipped on Linux it's hard to say if any regressions are being added by new code.<br><br></div>Here's a second attempt to fix this double completion bug. It works a bit differently since the whole command structure seems determined to keep adding on matches to the match list. This just removes the dups at the very end by adding a new function to StringsList to sort the list and remove the dups. Let me know what you think. If it looks reasonable please commit it. Thanks,<br><br>Index: include/lldb/Core/StringList.h<br>===================================================================<br>--- include/lldb/Core/StringList.h (revision 184143)<br>+++ include/lldb/Core/StringList.h (working copy)<br>@@ -72,6 +72,9 @@<br> void<br> RemoveBlankLines ();<br> <br>+ void<br>+ RemoveDupsAndSort ();<br>+<br> size_t<br> SplitIntoLines (const char *lines, size_t len);<br> <span class="Apple-converted-space"> </span><br>Index: source/Core/StringList.cpp<br>===================================================================<br>--- source/Core/StringList.cpp (revision 184143)<br>+++ source/Core/StringList.cpp (working copy)<br>@@ -230,6 +230,18 @@<br> }<br> }<br> <br>+// Remove any duplicates from the list of strings, side effect it will sort the list<br>+void<br>+StringList::RemoveDupsAndSort ()<br>+{<br>+ if (GetSize() < 2)<br>+ return;<br>+<br>+ // sort the list, remove the dups<br>+ std::sort(m_strings.begin(), m_strings.end());<br>+ m_strings.erase(std::unique(m_strings.begin(), m_strings.end()), m_strings.end());<br>+}<br>+<br> std::string<br> StringList::CopyList(const char* item_preamble,<br> const char* items_sep)<br>Index: source/Interpreter/CommandInterpreter.cpp<br>===================================================================<br>--- source/Interpreter/CommandInterpreter.cpp (revision 184143)<br>+++ source/Interpreter/CommandInterpreter.cpp (working copy)<br>@@ -1889,6 +1889,10 @@<br> {<br> // The cursor is in the first argument, so just do a lookup in the dictionary.<br> CommandObject *cmd_obj = GetCommandObject (parsed_line.GetArgumentAtIndex(0), &matches);<br>+<br>+ // Remove the dups<br>+ matches.RemoveDupsAndSort();<br>+ <span class="Apple-converted-space"> </span><br> num_command_matches = matches.GetSize();<br> <br> if (num_command_matches == 1<br><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 17, 2013 at 11:03 AM, Kopec, Matt<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:matt.kopec@intel.com" target="_blank">matt.kopec@intel.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div>My mistake, this was fixed in r183932 and I wasn't updated. However, the tests weren't updated to reflect this fix. I'll commit a fix for that.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Matt</div></font></span><div><div class="h5"><div><br></div><div><div><div>On 2013-06-14, at 5:20 PM, Matthew Sorrels <<a href="mailto:sorrels.m@gmail.com" target="_blank">sorrels.m@gmail.com</a>> wrote:</div><br><blockquote type="cite"><div dir="ltr"><div><div>If I fix the test so it has the space, I can reproduce your test failure with my patch. There were a lot of ways to fix this double printing bug, I just need to use one that doesn't also have a few side effects.<br><br></div>I synced and rebuilt lldb clean and I'm definitely getting a space on that line doing the TestAliases.py test. I wonder if it's some issue of the toolchain (version of Phython, version of clang, etc) I'm using. Here's the -t output on that section:<br><br>runCmd: bpi<br>output: Current breakpoints:<br>1: name = 'foo', locations = 1<br> 1.1: where = a.out`foo(int, int) + 14 at main.cpp:43, address = 0x000000000040070e, unresolved, hit count = 0<span class="Apple-converted-space"> </span><br><br>2: name = 'sum', locations = 1<br> 2.1: where = a.out`sum(int, int) + 8 at main.cpp:25, address = 0x0000000000400698, unresolved, hit count = 0<span class="Apple-converted-space"> </span><br><br>3: file = 'main.cpp', line = 32, locations = 1<br> 3.1: where = a.out`strange_max(int, int) + 8 at main.cpp:32, address = 0x00000000004006b8, unresolved, hit count = 0<span class="Apple-converted-space"> </span><br><br><br></div>Which fails the test as it's written since there is a space between the = and 'main.cpp' are you saying your lldb output doesn't have a space between the equal sign and 'main.cpp'?<br><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 14, 2013 at 1:58 PM, Kopec, Matt<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:matt.kopec@intel.com" target="_blank">matt.kopec@intel.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;">With my version of lldb, there is no space as you describe. I even checked the output manually in the command line. Strangely, I'm on a very recent lldb too. That will warrant separate investigation. Otherwise, you are running the test correctly. Can you reproduce?<div><br></div><div>Matt</div><div><br><div><div>On 2013-06-14, at 4:33 PM, Matthew Sorrels <<a href="mailto:sorrels.m@gmail.com" target="_blank">sorrels.m@gmail.com</a>></div><div><div> wrote:</div><br><blockquote type="cite"><div dir="ltr"><div>I get a failure in TestAliases.py without my change as well. But the error I get is different than what you list (occurs sooner at line 99). It appears to be a bug in the test script itself, it's missing a space. on line 99 between the = and 'main.cpp' Or am I running these tests wrong somehow? If I add the space the test runs (without my diff).<br><br></div>I can see why my patch is causing the regression your reporting. I think I'm going to have make the matches list remove dups (rather than just raw append).<br><div><br>matthews@matthews-linux:~/work/llvm/llvm/tools/lldb/test$ python dotest.py --executable ~/work/llvm/llvm/build/bin/lldb -q -p TestAliases.py<br>UNSUPPORTED: LLDB (clang-x86_64) :: test_with_dsym (TestAliases.AliasTestCase) (requires Darwin)<span class="Apple-converted-space"> </span><br>FAIL: LLDB (clang-x86_64) :: test_with_dwarf (TestAliases.AliasTestCase)<br>======================================================================<br>FAIL: test_with_dwarf (TestAliases.AliasTestCase)<br>----------------------------------------------------------------------<br>Traceback (most recent call last):<br> File "/data/work/llvm/llvm/tools/lldb.svn/test/lldbtest.py", line 368, in wrapper<br> return func(self, *args, **kwargs)<br> File "/data/work/llvm/llvm/tools/lldb.svn/test/functionalities/alias/TestAliases.py", line 24, in test_with_dwarf<br> self.alias_tests ()<br> File "/data/work/llvm/llvm/tools/lldb.svn/test/functionalities/alias/TestAliases.py", line 99, in alias_tests<br> "3: file ='main.cpp', line = 32, locations = 1" ])<br> File "/data/work/llvm/llvm/tools/lldb.svn/test/lldbtest.py", line 1720, in expect<br> msg if msg else EXP_MSG(str, exe))<br>AssertionError: False is not True : '3: file ='main.cpp', line = 32, locations = 1' returns expected result<br>Config=x86_64-clang<br>----------------------------------------------------------------------<br>Ran 2 tests in 0.443s<br><br>FAILED (failures=1, skipped=1)<br><br><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 14, 2013 at 12:15 PM, Kopec, Matt<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:matt.kopec@intel.com" target="_blank">matt.kopec@intel.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Hi Matthew,<br><br>I ran this patch against the lldb test suite and a regression came up. Here is how to quickly run the failing test and the corresponding output:<br><br>mkopec1@mkopec1-linux:~/dev/llvm/tools/lldb/test$ python dotest.py --executable /home/mkopec1/dev/llvm/build/Debug+Asserts/bin/lldb -q -p TestAliases.py<br>UNSUPPORTED: LLDB (clang-x86_64) :: test_with_dsym (TestAliases.AliasTestCase) (requires Darwin)<br>FAIL: LLDB (clang-x86_64) :: test_with_dwarf (TestAliases.AliasTestCase)<br>======================================================================<br>FAIL: test_with_dwarf (TestAliases.AliasTestCase)<br>----------------------------------------------------------------------<br>Traceback (most recent call last):<br> File "/home/mkopec1/dev/llvm/tools/lldb/test/lldbtest.py", line 368, in wrapper<br> return func(self, *args, **kwargs)<br> File "/home/mkopec1/dev/llvm/tools/lldb/test/functionalities/alias/TestAliases.py", line 24, in test_with_dwarf<br> self.alias_tests ()<br> File "/home/mkopec1/dev/llvm/tools/lldb/test/functionalities/alias/TestAliases.py", line 139, in alias_tests<br> self.runCmd ("alias exprf2 expr --raw -f %1 --")<br> File "/home/mkopec1/dev/llvm/tools/lldb/test/lldbtest.py", line 1582, in runCmd<br> msg if msg else CMD_MSG(cmd))<br>AssertionError: False is not True : Command 'alias exprf2 expr --raw -f %1 --' returns successfully<br>Config=x86_64-clang<br>----------------------------------------------------------------------<br>Ran 2 tests in 0.633s<br><br>FAILED (failures=1, skipped=1)<br><br>Matt<br><div><br>On 2013-06-07, at 5:24 PM, Matthew Sorrels <<a href="mailto:sorrels.m@gmail.com" target="_blank">sorrels.m@gmail.com</a>> wrote:<br><br>> When doing command completion with multiple command matches you will see the commands twice. The best example of this bug is typing pl<tab><br>><br>> (lldb) pl<br>> Available completions:<br>> platform<br>> plugin<br>> platform<br>> plugin<br>><br>><br>> with this patch you get the expected result:<br>><br>> (lldb) pl<br>> Available completions:<br>> platform<br>> plugin<br>><br>><br>> The fix works because once an exact match for a command and a command+aliases fails it just asks for any inexact matches from both commands and aliases rather than asking for any inexact commands (which returns a list of matches) and then any inexact commands and aliases (which adds the commands to the list again plus any aliases).<br>><br>> If this seems reasonable, please commit it for me. Thanks.<br>><br>><br>> Index: source/Interpreter/CommandInterpreter.cpp<br>> ===================================================================<br>> --- source/Interpreter/CommandInterpreter.cpp (revision 183564)<br>> +++ source/Interpreter/CommandInterpreter.cpp (working copy)<br>> @@ -848,13 +848,7 @@<br>> command_obj = GetCommandSP (cmd_cstr, true, true, matches).get();<br>> }<br>><br>> - // If there wasn't an exact match among the aliases, look for an inexact match<br>> - // in just the commands.<br>> -<br>> - if (command_obj == NULL)<br>> - command_obj = GetCommandSP(cmd_cstr, false, false, matches).get();<br>> -<br>> - // Finally, if there wasn't an inexact match among the commands, look for an inexact<br>> + // If there wasn't an exact match among the commands and aliases, look for an inexact<br>> // match in both the commands and aliases.<br>> if (command_obj == NULL)<br>> command_obj = GetCommandSP(cmd_cstr, true, false, matches).get();<br>><br></div>> _______________________________________________<br>> lldb-commits mailing list<br>><span class="Apple-converted-space"> </span><a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br>><span class="Apple-converted-space"> </span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br><br></blockquote></div><br></div></div></div></blockquote></div></div><br></div></div></blockquote></div><br></div></blockquote></div><br></div></div></div></div></blockquote></div><br></div>_______________________________________________<br>lldb-commits mailing list<br><a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a></div></blockquote></div><br></div></div></div></div></body></html>