https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/65974:
Previously we would check all built-ins first for suggestions, then check built-ins and aliases. This meant that if you had an alias brkpt -> breakpoint, "br" would complete to "breakpoint". Instead of giving you the choice of "brkpt" or "breakpoint". >From f1f7de482046fd9a1347d040827005841c4d863c Mon Sep 17 00:00:00 2001 From: David Spickett <david.spick...@linaro.org> Date: Mon, 11 Sep 2023 10:46:53 +0000 Subject: [PATCH 1/3] [lldb] Correctly invalidate unloaded image tokens Some functions in Process were using LLDB_INVALID_ADDRESS instead of LLDB_INVALID_TOKEN. The only visible effect of this appears to be that "process unload <tab>" would complete to 0 even after the image was unloaded. Since the command is checking for LLDB_INVALID_TOKEN. Everything else worked somehow. I've added a check to the existing load unload tests anyway. The tab completion cannot be checked as is, but when I make them more strict in a later patch it will be tested. --- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp | 2 +- lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp | 2 +- lldb/source/Target/Process.cpp | 4 ++-- .../API/functionalities/load_unload/TestLoadUnload.py | 9 +++++++++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index 1e91f2ccd198259..b4f1b76c39dbebf 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -949,7 +949,7 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process, Status PlatformPOSIX::UnloadImage(lldb_private::Process *process, uint32_t image_token) { const addr_t image_addr = process->GetImagePtrFromToken(image_token); - if (image_addr == LLDB_INVALID_ADDRESS) + if (image_addr == LLDB_INVALID_IMAGE_TOKEN) return Status("Invalid image token"); StreamString expr; diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index cbac14e2ccf7a92..88d543289a8470e 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -415,7 +415,7 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, Status PlatformWindows::UnloadImage(Process *process, uint32_t image_token) { const addr_t address = process->GetImagePtrFromToken(image_token); - if (address == LLDB_INVALID_ADDRESS) + if (address == LLDB_INVALID_IMAGE_TOKEN) return Status("invalid image token"); StreamString expression; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 2b0774588138881..f82ab05362fbee9 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -5911,12 +5911,12 @@ size_t Process::AddImageToken(lldb::addr_t image_ptr) { lldb::addr_t Process::GetImagePtrFromToken(size_t token) const { if (token < m_image_tokens.size()) return m_image_tokens[token]; - return LLDB_INVALID_ADDRESS; + return LLDB_INVALID_IMAGE_TOKEN; } void Process::ResetImageToken(size_t token) { if (token < m_image_tokens.size()) - m_image_tokens[token] = LLDB_INVALID_ADDRESS; + m_image_tokens[token] = LLDB_INVALID_IMAGE_TOKEN; } Address diff --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py index 7e8acfa3021acfc..2208e520f1d5612 100644 --- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py +++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py @@ -307,6 +307,15 @@ def run_lldb_process_load_and_unload_commands(self): patterns=["Unloading .* with index %s.*ok" % index], ) + # Confirm that we unloaded properly. + self.expect( + "image lookup -n a_function", + "a_function should not exist after unload", + error=True, + matching=False, + patterns=["1 match found"], + ) + self.runCmd("process continue") @expectedFailureAll(oslist=["windows"]) # breakpoint not hit >From f7082c3bf9ee5ef68713998352566a08d92869ab Mon Sep 17 00:00:00 2001 From: David Spickett <david.spick...@linaro.org> Date: Mon, 11 Sep 2023 09:09:31 +0000 Subject: [PATCH 2/3] [lldb] Improve completion tests * Assert no completions for tests that should not find completions. * Remove regex mode from complete_from_to, which was unused. This exposed bugs in 2 of the tests, target stop-hook and process unload. These were fixed in previous commits but couldn't be tested properly until this patch. --- .../Python/lldbsuite/test/lldbtest.py | 30 +++--- .../completion/TestExprCompletion.py | 92 +++++++++---------- .../completion/TestCompletion.py | 14 +-- 3 files changed, 62 insertions(+), 74 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 49355d61911837f..4f8bdcd7263dc35 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -2223,10 +2223,7 @@ def check_completion_with_desc( ) self.assertFalse(got_failure, error_msg) - def complete_exactly(self, str_input, patterns): - self.complete_from_to(str_input, patterns, True) - - def complete_from_to(self, str_input, patterns, turn_off_re_match=False): + def complete_from_to(self, str_input, patterns): """Test that the completion mechanism completes str_input to patterns, where patterns could be a pattern-string or a list of pattern-strings""" # Patterns should not be None in order to proceed. @@ -2254,21 +2251,18 @@ def complete_from_to(self, str_input, patterns, turn_off_re_match=False): for idx in range(1, num_matches + 1): compare_string += match_strings.GetStringAtIndex(idx) + "\n" + # If the singular pattern is the same as the input, assume that we + # shouldn't have any completions. + if len(patterns) == 1 and str_input == patterns[0] and num_matches: + self.fail("Expected no completions but got:\n" + compare_string) + for p in patterns: - if turn_off_re_match: - self.expect( - compare_string, - msg=COMPLETION_MSG(str_input, p, match_strings), - exe=False, - substrs=[p], - ) - else: - self.expect( - compare_string, - msg=COMPLETION_MSG(str_input, p, match_strings), - exe=False, - patterns=[p], - ) + self.expect( + compare_string, + msg=COMPLETION_MSG(str_input, p, match_strings), + exe=False, + substrs=[p], + ) def completions_match(self, command, completions): """Checks that the completions for the given command are equal to the diff --git a/lldb/test/API/commands/expression/completion/TestExprCompletion.py b/lldb/test/API/commands/expression/completion/TestExprCompletion.py index 3c354a3bce1a9b5..ada818989c789a1 100644 --- a/lldb/test/API/commands/expression/completion/TestExprCompletion.py +++ b/lldb/test/API/commands/expression/completion/TestExprCompletion.py @@ -29,34 +29,34 @@ def test_expr_completion(self): ) # Completing member functions - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooNoArgs", "expr some_expr.FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooWithArgs", "expr some_expr.FooWithArgsBar(" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooWithMultipleArgs", "expr some_expr.FooWithMultipleArgsBar(", ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooUnderscore", "expr some_expr.FooUnderscoreBar_()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooNumbers", "expr some_expr.FooNumbersBar1()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.StaticMemberMethod", "expr some_expr.StaticMemberMethodBar()", ) # Completing static functions - self.complete_exactly( + self.complete_from_to( "expr Expr::StaticMemberMethod", "expr Expr::StaticMemberMethodBar()" ) # Completing member variables - self.complete_exactly( + self.complete_from_to( "expr some_expr.MemberVariab", "expr some_expr.MemberVariableBar" ) @@ -94,43 +94,43 @@ def test_expr_completion(self): self.completions_contain("expr 1+", ["1+some_expr", "1+static_cast"]) # Test with spaces - self.complete_exactly( + self.complete_from_to( "expr some_expr .FooNoArgs", "expr some_expr .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr .FooNoArgs", "expr some_expr .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr .FooNoArgs", "expr some_expr .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr. FooNoArgs", "expr some_expr. FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr . FooNoArgs", "expr some_expr . FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr Expr :: StaticMemberMethod", "expr Expr :: StaticMemberMethodBar()" ) - self.complete_exactly( + self.complete_from_to( "expr Expr ::StaticMemberMethod", "expr Expr ::StaticMemberMethodBar()" ) - self.complete_exactly( + self.complete_from_to( "expr Expr:: StaticMemberMethod", "expr Expr:: StaticMemberMethodBar()" ) # Test that string literals don't break our parsing logic. - self.complete_exactly( + self.complete_from_to( 'expr const char *cstr = "some_e"; char c = *cst', 'expr const char *cstr = "some_e"; char c = *cstr', ) - self.complete_exactly( + self.complete_from_to( 'expr const char *cstr = "some_e" ; char c = *cst', 'expr const char *cstr = "some_e" ; char c = *cstr', ) # Requesting completions inside an incomplete string doesn't provide any # completions. - self.complete_exactly( + self.complete_from_to( 'expr const char *cstr = "some_e', 'expr const char *cstr = "some_e' ) @@ -139,110 +139,110 @@ def test_expr_completion(self): self.assume_no_completions("expr -i0 -- some_expr.", 11) # Test with expr arguments - self.complete_exactly( + self.complete_from_to( "expr -i0 -- some_expr .FooNoArgs", "expr -i0 -- some_expr .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr -i0 -- some_expr .FooNoArgs", "expr -i0 -- some_expr .FooNoArgsBar()", ) # Addrof and deref - self.complete_exactly( + self.complete_from_to( "expr (*(&some_expr)).FooNoArgs", "expr (*(&some_expr)).FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr (*(&some_expr)) .FooNoArgs", "expr (*(&some_expr)) .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr (* (&some_expr)) .FooNoArgs", "expr (* (&some_expr)) .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr (* (& some_expr)) .FooNoArgs", "expr (* (& some_expr)) .FooNoArgsBar()", ) # Addrof and deref (part 2) - self.complete_exactly( + self.complete_from_to( "expr (&some_expr)->FooNoArgs", "expr (&some_expr)->FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr (&some_expr) ->FooNoArgs", "expr (&some_expr) ->FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr (&some_expr) -> FooNoArgs", "expr (&some_expr) -> FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr (&some_expr)-> FooNoArgs", "expr (&some_expr)-> FooNoArgsBar()" ) # Builtin arg - self.complete_exactly("expr static_ca", "expr static_cast") + self.complete_from_to("expr static_ca", "expr static_cast") # From other files - self.complete_exactly( + self.complete_from_to( "expr fwd_decl_ptr->Hidden", "expr fwd_decl_ptr->HiddenMember" ) # Types - self.complete_exactly("expr LongClassNa", "expr LongClassName") - self.complete_exactly( + self.complete_from_to("expr LongClassNa", "expr LongClassName") + self.complete_from_to( "expr LongNamespaceName::NestedCla", "expr LongNamespaceName::NestedClass" ) # Namespaces - self.complete_exactly("expr LongNamespaceNa", "expr LongNamespaceName::") + self.complete_from_to("expr LongNamespaceNa", "expr LongNamespaceName::") # Multiple arguments - self.complete_exactly( + self.complete_from_to( "expr &some_expr + &some_e", "expr &some_expr + &some_expr" ) - self.complete_exactly( + self.complete_from_to( "expr SomeLongVarNameWithCapitals + SomeLongVarName", "expr SomeLongVarNameWithCapitals + SomeLongVarNameWithCapitals", ) - self.complete_exactly( + self.complete_from_to( "expr SomeIntVar + SomeIntV", "expr SomeIntVar + SomeIntVar" ) # Multiple statements - self.complete_exactly( + self.complete_from_to( "expr long LocalVariable = 0; LocalVaria", "expr long LocalVariable = 0; LocalVariable", ) # Custom Decls - self.complete_exactly( + self.complete_from_to( "expr auto l = [](int LeftHandSide, int bx){ return LeftHandS", "expr auto l = [](int LeftHandSide, int bx){ return LeftHandSide", ) - self.complete_exactly( + self.complete_from_to( "expr struct LocalStruct { long MemberName; } ; LocalStruct S; S.Mem", "expr struct LocalStruct { long MemberName; } ; LocalStruct S; S.MemberName", ) # Completing function call arguments - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooWithArgsBar(some_exp", "expr some_expr.FooWithArgsBar(some_expr", ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooWithArgsBar(SomeIntV", "expr some_expr.FooWithArgsBar(SomeIntVar", ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.FooWithMultipleArgsBar(SomeIntVar, SomeIntVa", "expr some_expr.FooWithMultipleArgsBar(SomeIntVar, SomeIntVar", ) # Function return values - self.complete_exactly( + self.complete_from_to( "expr some_expr.Self().FooNoArgs", "expr some_expr.Self().FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.Self() .FooNoArgs", "expr some_expr.Self() .FooNoArgsBar()" ) - self.complete_exactly( + self.complete_from_to( "expr some_expr.Self(). FooNoArgs", "expr some_expr.Self(). FooNoArgsBar()" ) diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py index cc2a20dcd0dca76..8898bf2e2b5da9e 100644 --- a/lldb/test/API/functionalities/completion/TestCompletion.py +++ b/lldb/test/API/functionalities/completion/TestCompletion.py @@ -237,16 +237,13 @@ def test_log_dir(self): src_dir = os.path.dirname(os.path.realpath(__file__)) self.complete_from_to( "log enable lldb expr -f " + src_dir, - [src_dir + os.sep], - turn_off_re_match=True, - ) + [src_dir + os.sep]) # <rdar://problem/11052829> def test_infinite_loop_while_completing(self): """Test that 'process print hello\' completes to itself and does not infinite loop.""" self.complete_from_to( - "process print hello\\", "process print hello\\", turn_off_re_match=True - ) + "process print hello\\", "process print hello\\") def test_watchpoint_co(self): """Test that 'watchpoint co' completes to 'watchpoint command '.""" @@ -727,9 +724,7 @@ def test_symbol_name(self): self.dbg.CreateTarget(self.getBuildArtifact("a.out")) self.complete_from_to( "breakpoint set -n Fo", - "breakpoint set -n Foo::Bar(int,\\ int)", - turn_off_re_match=True, - ) + "breakpoint set -n Foo::Bar(int,\\ int)") # No completion for Qu because the candidate is # (anonymous namespace)::Quux(). self.complete_from_to("breakpoint set -n Qu", "") @@ -822,8 +817,7 @@ def test_common_completion_target_stophook_ids(self): for subcommand in subcommands: self.complete_from_to( "target stop-hook " + subcommand + " 1 ", - "target stop-hook " + subcommand + " 1 ", - ) + "target stop-hook " + subcommand + " 1 ") def test_common_completion_type_language(self): self.complete_from_to("type category -l ", ["c"]) >From 085393af7b13ffd9eac87ac28788663f22dfe7cf Mon Sep 17 00:00:00 2001 From: David Spickett <david.spick...@linaro.org> Date: Mon, 11 Sep 2023 11:42:45 +0000 Subject: [PATCH 3/3] [lldb] Treat user aliases the same as built-ins when tab completing Previously we would check all built-ins first for suggestions, then check built-ins and aliases. This meant that if you had an alias brkpt -> breakpoint, "br" would complete to "breakpoint". Instead of giving you the choice of "brkpt" or "breakpoint". --- .../source/Interpreter/CommandInterpreter.cpp | 35 +++---------------- .../abbreviation/TestAbbreviations.py | 2 +- .../completion/TestCompletion.py | 18 ++++------ 3 files changed, 13 insertions(+), 42 deletions(-) diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 6d1ad799f2d10fb..fc94bf6fbfe117c 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -1227,36 +1227,11 @@ CommandObject * CommandInterpreter::GetCommandObject(llvm::StringRef cmd_str, StringList *matches, StringList *descriptions) const { - CommandObject *command_obj = - GetCommandSP(cmd_str, false, true, matches, descriptions).get(); - - // If we didn't find an exact match to the command string in the commands, - // look in the aliases. - - if (command_obj) - return command_obj; - - command_obj = GetCommandSP(cmd_str, true, true, matches, descriptions).get(); - - if (command_obj) - return command_obj; - - // If there wasn't an exact match then look for an inexact one in just the - // commands - command_obj = GetCommandSP(cmd_str, false, false, nullptr).get(); - - // Finally, if there wasn't an inexact match among the commands, look for an - // inexact match in both the commands and aliases. - - if (command_obj) { - if (matches) - matches->AppendString(command_obj->GetCommandName()); - if (descriptions) - descriptions->AppendString(command_obj->GetHelp()); - return command_obj; - } - - return GetCommandSP(cmd_str, true, false, matches, descriptions).get(); + // Try to find a match among commands and aliases. Allowing inexact matches, + // but perferring exact matches. + return GetCommandSP(cmd_str, /*include_aliases=*/true, /*exact=*/false, + matches, descriptions) + .get(); } CommandObject *CommandInterpreter::GetUserCommandObject( diff --git a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py index 10431e41dc81a2e..cade8d87e7f76f5 100644 --- a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py +++ b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py @@ -20,7 +20,7 @@ def test_command_abbreviations_and_aliases(self): self.assertTrue(result.Succeeded()) self.assertEqual("apropos script", result.GetOutput()) - command_interpreter.ResolveCommand("h", result) + command_interpreter.ResolveCommand("he", result) self.assertTrue(result.Succeeded()) self.assertEqual("help", result.GetOutput()) diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py index 8898bf2e2b5da9e..fd1db08b9995c88 100644 --- a/lldb/test/API/functionalities/completion/TestCompletion.py +++ b/lldb/test/API/functionalities/completion/TestCompletion.py @@ -621,19 +621,15 @@ def test_command_unalias(self): def test_command_aliases(self): self.runCmd("command alias brkpt breakpoint") - # If there is an unambiguous completion from the built-in commands, - # we choose that. - self.complete_from_to("br", "breakpoint") - # Only if there is not, do we then look for an unambiguous completion - # from the user defined aliases. + # Exact matches are chosen if possible, even if there are longer + # completions we could use. + self.complete_from_to("b", "b ") + # Aliases are included in possible completions. + self.complete_from_to("br", ["breakpoint", "brkpt"]) + # An alias can be the chosen completion. self.complete_from_to("brk", "brkpt") - # Aliases are included when there's no exact match. - self.runCmd("command alias play breakpoint") - self.complete_from_to("pl", ["plugin", "platform", "play"]) - - # That list can also contain only aliases if there's no built-ins to - # match. + # The list can contain only aliases if there's no built-ins to match. self.runCmd("command alias test_1 breakpoint") self.runCmd("command alias test_2 breakpoint") self.complete_from_to("test_", ["test_1", "test_2"]) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits