[Lldb-commits] Fix for double command completion text
Matthew Sorrels
sorrels.m at gmail.com
Mon Jun 17 16:32:54 PDT 2013
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.
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,
Index: include/lldb/Core/StringList.h
===================================================================
--- include/lldb/Core/StringList.h (revision 184143)
+++ include/lldb/Core/StringList.h (working copy)
@@ -72,6 +72,9 @@
void
RemoveBlankLines ();
+ void
+ RemoveDupsAndSort ();
+
size_t
SplitIntoLines (const char *lines, size_t len);
Index: source/Core/StringList.cpp
===================================================================
--- source/Core/StringList.cpp (revision 184143)
+++ source/Core/StringList.cpp (working copy)
@@ -230,6 +230,18 @@
}
}
+// Remove any duplicates from the list of strings, side effect it will
sort the list
+void
+StringList::RemoveDupsAndSort ()
+{
+ if (GetSize() < 2)
+ return;
+
+ // sort the list, remove the dups
+ std::sort(m_strings.begin(), m_strings.end());
+ m_strings.erase(std::unique(m_strings.begin(), m_strings.end()),
m_strings.end());
+}
+
std::string
StringList::CopyList(const char* item_preamble,
const char* items_sep)
Index: source/Interpreter/CommandInterpreter.cpp
===================================================================
--- source/Interpreter/CommandInterpreter.cpp (revision 184143)
+++ source/Interpreter/CommandInterpreter.cpp (working copy)
@@ -1889,6 +1889,10 @@
{
// The cursor is in the first argument, so just do a lookup in the
dictionary.
CommandObject *cmd_obj = GetCommandObject
(parsed_line.GetArgumentAtIndex(0), &matches);
+
+ // Remove the dups
+ matches.RemoveDupsAndSort();
+
num_command_matches = matches.GetSize();
if (num_command_matches == 1
On Mon, Jun 17, 2013 at 11:03 AM, Kopec, Matt <matt.kopec at intel.com> wrote:
> 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.
>
> Matt
>
> On 2013-06-14, at 5:20 PM, Matthew Sorrels <sorrels.m at gmail.com> wrote:
>
> 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.
>
> 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:
>
> runCmd: bpi
> output: Current breakpoints:
> 1: name = 'foo', locations = 1
> 1.1: where = a.out`foo(int, int) + 14 at main.cpp:43, address =
> 0x000000000040070e, unresolved, hit count = 0
>
> 2: name = 'sum', locations = 1
> 2.1: where = a.out`sum(int, int) + 8 at main.cpp:25, address =
> 0x0000000000400698, unresolved, hit count = 0
>
> 3: file = 'main.cpp', line = 32, locations = 1
> 3.1: where = a.out`strange_max(int, int) + 8 at main.cpp:32, address =
> 0x00000000004006b8, unresolved, hit count = 0
>
>
> 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'?
>
>
>
> On Fri, Jun 14, 2013 at 1:58 PM, Kopec, Matt <matt.kopec at intel.com> wrote:
>
>> 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?
>>
>> Matt
>>
>> On 2013-06-14, at 4:33 PM, Matthew Sorrels <sorrels.m at gmail.com>
>> wrote:
>>
>> 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).
>>
>> 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).
>>
>> matthews at matthews-linux:~/work/llvm/llvm/tools/lldb/test$ python
>> dotest.py --executable ~/work/llvm/llvm/build/bin/lldb -q -p TestAliases.py
>> UNSUPPORTED: LLDB (clang-x86_64) :: test_with_dsym
>> (TestAliases.AliasTestCase) (requires Darwin)
>> FAIL: LLDB (clang-x86_64) :: test_with_dwarf (TestAliases.AliasTestCase)
>> ======================================================================
>> FAIL: test_with_dwarf (TestAliases.AliasTestCase)
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>> File "/data/work/llvm/llvm/tools/lldb.svn/test/lldbtest.py", line 368,
>> in wrapper
>> return func(self, *args, **kwargs)
>> File
>> "/data/work/llvm/llvm/tools/lldb.svn/test/functionalities/alias/TestAliases.py",
>> line 24, in test_with_dwarf
>> self.alias_tests ()
>> File
>> "/data/work/llvm/llvm/tools/lldb.svn/test/functionalities/alias/TestAliases.py",
>> line 99, in alias_tests
>> "3: file ='main.cpp', line = 32, locations = 1" ])
>> File "/data/work/llvm/llvm/tools/lldb.svn/test/lldbtest.py", line 1720,
>> in expect
>> msg if msg else EXP_MSG(str, exe))
>> AssertionError: False is not True : '3: file ='main.cpp', line = 32,
>> locations = 1' returns expected result
>> Config=x86_64-clang
>> ----------------------------------------------------------------------
>> Ran 2 tests in 0.443s
>>
>> FAILED (failures=1, skipped=1)
>>
>>
>>
>>
>> On Fri, Jun 14, 2013 at 12:15 PM, Kopec, Matt <matt.kopec at intel.com>wrote:
>>
>>> Hi Matthew,
>>>
>>> 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:
>>>
>>> mkopec1 at mkopec1-linux:~/dev/llvm/tools/lldb/test$ python dotest.py
>>> --executable /home/mkopec1/dev/llvm/build/Debug+Asserts/bin/lldb -q -p
>>> TestAliases.py
>>> UNSUPPORTED: LLDB (clang-x86_64) :: test_with_dsym
>>> (TestAliases.AliasTestCase) (requires Darwin)
>>> FAIL: LLDB (clang-x86_64) :: test_with_dwarf (TestAliases.AliasTestCase)
>>> ======================================================================
>>> FAIL: test_with_dwarf (TestAliases.AliasTestCase)
>>> ----------------------------------------------------------------------
>>> Traceback (most recent call last):
>>> File "/home/mkopec1/dev/llvm/tools/lldb/test/lldbtest.py", line 368,
>>> in wrapper
>>> return func(self, *args, **kwargs)
>>> File
>>> "/home/mkopec1/dev/llvm/tools/lldb/test/functionalities/alias/TestAliases.py",
>>> line 24, in test_with_dwarf
>>> self.alias_tests ()
>>> File
>>> "/home/mkopec1/dev/llvm/tools/lldb/test/functionalities/alias/TestAliases.py",
>>> line 139, in alias_tests
>>> self.runCmd ("alias exprf2 expr --raw -f %1 --")
>>> File "/home/mkopec1/dev/llvm/tools/lldb/test/lldbtest.py", line 1582,
>>> in runCmd
>>> msg if msg else CMD_MSG(cmd))
>>> AssertionError: False is not True : Command 'alias exprf2 expr --raw -f
>>> %1 --' returns successfully
>>> Config=x86_64-clang
>>> ----------------------------------------------------------------------
>>> Ran 2 tests in 0.633s
>>>
>>> FAILED (failures=1, skipped=1)
>>>
>>> Matt
>>>
>>> On 2013-06-07, at 5:24 PM, Matthew Sorrels <sorrels.m at gmail.com> wrote:
>>>
>>> > When doing command completion with multiple command matches you will
>>> see the commands twice. The best example of this bug is typing pl<tab>
>>> >
>>> > (lldb) pl
>>> > Available completions:
>>> > platform
>>> > plugin
>>> > platform
>>> > plugin
>>> >
>>> >
>>> > with this patch you get the expected result:
>>> >
>>> > (lldb) pl
>>> > Available completions:
>>> > platform
>>> > plugin
>>> >
>>> >
>>> > 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).
>>> >
>>> > If this seems reasonable, please commit it for me. Thanks.
>>> >
>>> >
>>> > Index: source/Interpreter/CommandInterpreter.cpp
>>> > ===================================================================
>>> > --- source/Interpreter/CommandInterpreter.cpp (revision 183564)
>>> > +++ source/Interpreter/CommandInterpreter.cpp (working copy)
>>> > @@ -848,13 +848,7 @@
>>> > command_obj = GetCommandSP (cmd_cstr, true, true,
>>> matches).get();
>>> > }
>>> >
>>> > - // If there wasn't an exact match among the aliases, look for an
>>> inexact match
>>> > - // in just the commands.
>>> > -
>>> > - if (command_obj == NULL)
>>> > - command_obj = GetCommandSP(cmd_cstr, false, false,
>>> matches).get();
>>> > -
>>> > - // Finally, if there wasn't an inexact match among the commands,
>>> look for an inexact
>>> > + // If there wasn't an exact match among the commands and aliases,
>>> look for an inexact
>>> > // match in both the commands and aliases.
>>> > if (command_obj == NULL)
>>> > command_obj = GetCommandSP(cmd_cstr, true, false,
>>> matches).get();
>>> >
>>> > _______________________________________________
>>> > lldb-commits mailing list
>>> > lldb-commits at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130617/e188aaf0/attachment.html>
More information about the lldb-commits
mailing list